php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #69207 move_uploaded_file allows nulls in path
Submitted: 2015-03-09 17:35 UTC Modified: 2015-03-31 05:45 UTC
From: habte dot yibelo at gmail dot com Assigned: stas
Status: Closed Package: PHP Language Specification
PHP Version: Irrelevant OS: *
Private report: No CVE-ID: 2015-2348
 [2015-03-09 17:35 UTC] habte dot yibelo at gmail dot com
Description:
------------
with newer versions of PHP, we all taught Nullbytes issues were over. but there is still one simple bypass and I still have no clue why there is.



Test script:
---------------
move_uploaded_file($_FILES['x']['tmp_name'],"/tmp/test.php\x00.jpg")

That file will create what you think it shouldn't!

Actual result:
--------------
I expect the fix of the next version to notice nullbytes and actually not ignore them this time.

Thanks!

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-03-09 18:12 UTC] stas@php.net
Could you please provide more information about what you think is the problem, what is the expected result and what is the actual result?
 [2015-03-09 18:21 UTC] habte dot yibelo at gmail dot com
Hi, @stats

Sure. why not. the problem is that, the null byte is deprecated (I think to stop LFI... and similar attacks). Nullbytes are dangerous and thus, are not actually avilable since php 5.3.1, however, using \x00 it is still possible to use them.

say the script:

move_uploaded_file($_FILES['x']['tmp_name'],"/tmp/test.php\x00.jpg")

Should actually have created /tmp/test.php\x00.jpg when in reality, it creates /tmp/test.php which should bypass .jpg extenssion checking scripts. I still don't know why this nullbyte works when the %00 is deprecated.

Anyway, I guess I have answered your question
 [2015-03-17 19:47 UTC] stas@php.net
-Summary: Nullbytes - The Come back +Summary: move_uploaded_file allows nulls in path
 [2015-03-17 23:39 UTC] stas@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: stas
 [2015-03-17 23:39 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2015-03-18 05:09 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=1291d6bbee93b6109eb07e8f7916ff1b7fcc13e1
Log: Fix bug #69207 - move_uploaded_file allows nulls in path
 [2015-03-18 09:47 UTC] jpauli@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=93b8970b3316cd5c557eae2f03d7efc39a9eae53
Log: Fix bug #69207 - move_uploaded_file allows nulls in path
 [2015-03-19 01:00 UTC] tyrael@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=313ff116fa08918fb3949d51f17eb39d0b950962
Log: Fix bug #69207 - move_uploaded_file allows nulls in path
 [2015-03-20 14:25 UTC] habte dot yibelo at gmail dot com
Hi, Again.

This has been assigned with CVE-2015-2348. it will be published in mitre as soon as a fix is publicly available (if it hasn't already) 5.3.7, maybe?

This has been classified as remotely exploitable  modernate-critical issue following the reason(s):

- If you allow file uploads and you check them on extension, its very probable you pass the file using move_uploaded_file and thus causing an RCE.
- Checking on mime type, size, content header, extension... won't stop the exploit from passing as a valid file. consider the code https://github.com/RandomStorm/DVWA/blob/master/vulnerabilities/upload/source/high.php that code is suppose to be a secure/unbreakable code for RCE. its the highest level of DVWA. many developers follow the same practice and I am sure no one still blacklists null-bytes.

Thanks for pushing a fix so fast! :)
 [2015-03-20 17:57 UTC] stas@php.net
I'm not sure how you achieve remote exploit with this, unless you already have security vulnerability, such as allowing writing arbitrary remotely specified files or trusting extension supplied externally. Trusting remotely supplied extension is a grave security error as naturally you can be given any file under any extension, regardless of its contents. DWVA is an example of this error, but nobody should be actually writing such code if they want any security.
 [2015-03-20 18:09 UTC] habte dot yibelo at gmail dot com
"... but nobody should be actually writing such code if they want any security."

Indeed. but most of the codes I audit have similar checks. extension & mime checking, its how everybody does it. and I see no problems with it, (at-least to cause RCE) until this bug. I agree with you that this like you said won't happen if the name got randomized and explicitly moved to a custom extension.

But considering how many sites are out there with such vulnerabilities like the one in DVWA, this is remotely exploitable. but there is no use in debating while this issue has been fixed! :)

Thanks!
 [2015-03-30 08:03 UTC] kaplan@php.net
I can't validate the CVE you've mentioned. It does appear as assigned, but no details (or any PHP reference) although the fix is public for more than 10 days. I'm going to ignore it for now.
 [2015-03-30 08:17 UTC] habte dot yibelo at gmail dot com
@kaplan Yes I know. I have been trying to reach the CVE team for days. they haven't provided any public info just yet. I don't know what is wrong with them but I am hoping they will release public info soon.

Thanks!
 [2015-03-31 05:45 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: 2015-2348
 [2016-07-20 11:39 UTC] davey@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6632684e032e2a895c4fd6178c4bc3736d57e03c
Log: Fix bug #69207 - move_uploaded_file allows nulls in path
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Feb 26 14:01:37 2017 UTC