Skip to content
This repository has been archived by the owner on Sep 8, 2023. It is now read-only.

Question/Security - Execution of upload files. #1098

Open
Hexife opened this issue Nov 11, 2018 · 8 comments
Open

Question/Security - Execution of upload files. #1098

Hexife opened this issue Nov 11, 2018 · 8 comments

Comments

@Hexife
Copy link

Hexife commented Nov 11, 2018

Hello,

I have a question about management of uploaded file in your application.

Is it intended behavior that uploaded files are executable?

I checked that Codiad has not the mitigations for execution of uploaded file.

Even though uploading feature needs administrator credential, execution of uploaded file is still dangerous.

I think it needs to the mitigations like compressing/encoding upload file or hide the upload path.

@Hexife Hexife changed the title Security - Execution of upload files. Question/Security - Execution of upload files. Nov 11, 2018
@Hexife
Copy link
Author

Hexife commented Nov 22, 2018

I requested CVE for this vulnerability. CVE-2018-19423.

@mitchellurgero
Copy link

You mean the file is accessible by public web? that's intended. However if using apache you can simply put a .htaccess file with:

Deny from All

in it in the workspace folder to deny access to the folder from outside of PHP.

@Hexife
Copy link
Author

Hexife commented Jan 8, 2019

I'm glad to read your comment to this issue. 😄.

Even it was the intended feature, it seems to need the ways for preventing compromise this application and system. (Maybe implementation of php-sandbox is the one of the ways). Because of that point, I decided this to request CVEs.

Also, .htaccess is not a mitigation for this problem. Because Codiad allows the authenticated user to upload arbitrary .htaccess file, too. It overwrites the existed .htaccess file.

@mitchellurgero
Copy link

mitchellurgero commented Jan 8, 2019

You have to be signed in to upload though.

With your logic, every system out there is at risk in this way (and it is, if you don't take proper security measures).

.htaccess is sufficient if you trust your users, and if you don't trust them, then why do they have access to such a powerful system in the first place?

Though I get the worry. Any files uploaded into the apache root folder (default /var/www/html) can be accessed without using a htaccess file to deny access.

"You can edit the htaccess file though through codiad"

Then add the bits to /etc/apache2/apache2.conf, since that wont be writable by www-data:

<Directory /var/www/html/codiad/workspace>
    Deny from All
    AllowOverride None
</Directory>

"But I am using NGINX"

Then in /etc/nginx/sites-enabled/default (Or other site conf file) in the server directive put:

location /var/www/html/codiad/workspace {
    deny all;
    return 403;
}

You can tweak the above to allow only certain IP Address's access as well. To allow you to test the site you are building in Codiad but deny those who would otherwise try to exploit it in some way.

Also, you can use the above configurations without enabling rewrite (htaccess) to prevent users from using their own .htaccess.


The solution here is not a codiad issue. there shouldn't be a CVE for it unless there is code specific to codiad (In this case there is not btw).

Example: Install wordpress, upload a php file using the media uploader, access php file from wp-content/uploads/ folder. Same result. However on a properly secured server, that folder is not allowed access when it comes to php files. but that's not wordpress's job to secure YOUR server. You know?
(Though maybe wordpess is a bad example as I think they filter uploads unless you are an admin)

@Hexife
Copy link
Author

Hexife commented Jan 9, 2019

Hmm.. I decided to request the status of CVE as disputed because you said it is the intended behavior and I can't address that all kind of this behavior is vulnerable (other example is announced in About WordPress and other applications I written below).

But It still seems to expose to the CSRF attacks.
(If user who logged in Codiad visit malicious page that has CSRF payloads, payloads can upload arbitrary codes to them. Tested code is in below.)

<html>
<head>
<title>Codiad CSRF Test</title>
</head>
<body>
<form action="http://[IP]/codiad/components/filemanager/controller.php?
action=upload&path=test" method="POST" enctype="multipart/form-data">
<input type="file" id="csrftest" name="upload[]">
<input type="submit">
</form>
</body>
</html>

And here is some of my opinions for your comment.


  • The feature is provided for authorized user.

But it is not mean that authorized users always trusted. Sometimes user runs some unintended behavior by CSRF attacks, or is stolen their login session by XSS attacks. Even the feature that needs to some login authority, developers should implement the mitigation for these attacks.

In the same context, upload of .htaccess file is still dangerous even the authorized user try that.


  • It can be secured via server configurations.

I agree with your words. But I think the application should present the secure ways and It is not good that application depends on server configuration for their secure behavior.
Some service administrator install and service the web application in default server options because of some reasons(Inexperienced or lazy or compatibility with other apps etc..).
Therefore, some restriction needed to applications for preventing the vulnerabilities.

( Some apps involved .htaccess files in their projects for their apps. )


  • About WordPress and other applications.

I have researching for upload features that each web application have, and I already tested WordPress.
WordPress didn't allow the admin user to upload the php scripts via Media feature. ( WordPress admin user can edit theme written with php script on their admin panel, but it was out of scope in my test. Also, it affected decisively to my decisions that cancel the CVE for Codiad . )
HTML and JS file is allowed in Media feature, but they provide specific option that reject to upload the HTML and JS file by admin user. REFERENCE is here.

Also, Many popular web applications that have file upload feature has the file restriction policy for their web apps. ( I tested more then 20 web applications, and only 2 applications except Codiad did not have file restriction methods. )


Thank you for discussion with me about this issue.

@mitchellurgero
Copy link

It always good to have conversations like these! It's always better to be safe, than to be sorry, right?

Sometimes user runs some unintended behavior by CSRF attacks

I would submit a CVE after a dev gets a chance to resolve the matter (if they can, IDK who is managing the project at this time).

some restriction needed to applications for preventing the vulnerabilities.

I don't disagree with this, however it's still up to the server admin to be sure to use secure software and a secure configuration at the end of the day. You can't blame an app for stolen or otherwise compromised data. BUT an app that can help secure itself is ALWAYS welcome, I believe.

Many popular web applications that have file upload feature has the file restriction policy for their web apps.

I don't disagree with this either. It's good practice to build a secure application. However maybe my example was bad. Codiad is an IDE (Integrated Development Environment, as you know) so it needs access to more sensitive files (PHP, C, BAT, BASH, etc, etc). It's possible (and I am just a user of Codiad, not a dev) that the devs didn't restrict this because it doesn't make sense to restrict it? Maybe allowing a file restriction config (like in config.php maybe) to only allow certain file types would benefit this.


At the end of the day however, whether you are inexperienced, or just plain laziness, it will always be a server admins job to secure the server. Even if that means not using Codiad or XYZ software.

I appreciate you having a valid and calm discussion on this. Not many people these days can do that, so thank you!

@mitchellurgero
Copy link

I just forked Codiad https://github.com/mitchellurgero/Codiad

It will fix this as well as many other tiny things I have seen.

@NicoleG25
Copy link

Hello @Fluidbyte ,
Was this issue ever addressed? please note that there was a CVE assigned.

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

No branches or pull requests

3 participants