Opened 12 years ago
Closed 7 years ago
#5041 closed enhancement (maybelater)
Avatar uploads should have their MIME types checked more aggressively
Reported by: | lagdonkey | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 1.7 |
Component: | Core | Keywords: | needs-patch, close, trac-tidy-2018 |
Cc: |
Description
As per my thread at: http://buddypress.org/support/topic/a-potential-security-issue-with-avatar-uploads/
First off, I’m using BP 1.7.2, and wordpress 3.5.1 website is amazinglyamusing.com.
Just installed BP, and am about to start working on templates to fit it into my theme, however 1 criteria I require, is for users to be able to upload their own avatars, which BP does.
First off, I tested this feature on my localhost development site, and the first thing I tried was to break any security features it has. What I found was, I could easily take a standard raw PHP file, change the extension to .jpg, and it would upload. Of course it gave an error when it got to the cropping section, however the file is sitting in the folder wp-content/uploads/avatar/3. This is a MAJOR security issue, as anyone could very easily upload any malicious file and do what they want, if they can figure out where the uploaded files go(which wouldn’t be all that hard).
I’m just wondering if there’s some setting in BP itself I’m missing, or if this is really how this plugin works. I’ll admit, I don’t know all the ins and outs of web development and security, but this seems pretty dangerous, unless I’m missing something. It was my assumption that DP should be checking MIME filetype, and using other checksums to ensure this sort of thing can’t happen.
Change History (4)
#1
@
12 years ago
- Milestone changed from Awaiting Review to Future Release
- Summary changed from Possible security issue with avatar uploads. to Avatar uploads should have their MIME types checked more aggressively
- Type changed from defect (bug) to enhancement
#2
@
8 years ago
- Keywords close added
As we rely on WordPress functionality (and BuddyPress itself is just extending WP) in this field, I propose to close this ticket.
Once WordPress will introduce in its functions a more reliable solution - we will start instantly using it.
Also, take into account several server specific settings, that should be set to make this problem a serous one.
finfo()
has its issues, not 100% reliable, so not sure that using it will help a lot.
#3
@
7 years ago
- Keywords trac-tidy-2018 added
We're closing this ticket because it has not received any contribution or comments for at least two years. We have decided that it is better to close tickets that are good ideas, which have not gotten (or are unlikely to get) contributions, rather than keep things open indefinitely. This will help us share a more realistic roadmap for BuddyPress with you.
Everyone very much appreciates the time and effort that you spent sharing your idea with us. On behalf of the entire BuddyPress team, thank you.
If you feel strongly that this enhancement should still be added to BuddyPress, and you are able to contribute effort towards it, we encourage you to re-open the ticket, or start a discussion about it in our Slack channel. Please consider that time has proven that good ideas without contributions do not get built.
For more information, see https://bpdevel.wordpress.com/2018/01/21/our-awaiting-contributions-milestone-contains/
or find us on Slack, in the #buddypress channel: https://make.wordpress.org/chat/
Thanks for the report, lagdonkey.
You are correct that BP only checks uploads based on filenames, which can be spoofed. This is not ideal. However, a few points:
wp_check_filetype_and_ext()
. It determines mime type from filename, and only in the case of an image with an incorrect extension (like, you rename foo.jpg as foo.gif) does it actually do anything more sophisticated. See http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/functions.php#L1805wp_handle_upload()
, which BP uses to place the file in its permanent location, sets the file permissions of the uploaded file to 666. (http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/functions.php#L1737) Thus, even if someone were to upload a malicious executable file, it should not in fact be executable..jpg
etc cannot be parsed by PHP.Real MIME checks in PHP are generally done using
finfo
, but this extension is only available in PHP > 5.3, which is above BP/WP's current minimum requirements.So, for the moment, I'm going to put this in Future Release, with the expectation that we (or, more likely, WordPress) will harden it when the PHP 5.2.x series is dropped. In the meantime, if you are able to identify steps to a specific exploit - that is, not only can you upload such a file, but you can then execute arbitrary code through the browser - please send details to security@…
Thanks again for your report.