Home » HTTP » HTTP_Upload » Bug #2253

[2004-08-30 15:46 UTC] znoob2 at yahoo dot com

Description:
------------
HTTP_Upload version 0.9.1

I got a form on a page with a file input field named "f". I upload a file called (without the < and >):
<BI' evil!!.torrent>

Then when I check the name, the first part until the single quote gets ripped off.

Reproduce code:
---------------
// The script that shows things are nasty:

$upload = new HTTP_Upload( 'en' );
$torrent_file = $upload->getFiles( 'f' );
print_r( $torrent_file );

Expected result:
----------------
/* Expected (part of) output:
...
[upload] => Array
(
[real] => BI' evil!!.torrent
[name] => BI' evil!!.torrent
...
*/

Actual result:
--------------
/* Actual (part of) output:
...
[upload] => Array
(
[real] => ' evil!!.torrent
[name] => ' evil!!.torrent
...
*/

[2004-08-31 15:36 UTC] znoob2 at yahoo dot com

To reply to wenz:
no, $_FILES['f']['name'] returns the filename correctly apart from that it is addslashes-ed. But that is overcomeable.

And for antonio:
Sorry, but I don't understand a thing from that backtrace-making-tutorial. Especially because I am running WinXP and can't find those executables (unlimit, gdb). And how do I know if I compiled with --enable-debug? I downloaded the precompiled binaries.

Thanks for the support so far.

[2004-08-31 15:58 UTC] znoob2 at yahoo dot com

$_FILES:

Array
(
[f] => Array
(
[name] => BI\' evil!!.torrent
[type] => application/octet-stream
[tmp_name] => C:\WINDOWS\TEMP\php7.tmp
[error] => 0
[size] => 0
)

)

and $HTTP_POST_FILES:

Array
(
[f] => Array
(
[name] => BI\' evil!!.torrent
[type] => application/octet-stream
[tmp_name] => C:\WINDOWS\TEMP\php7.tmp
[error] => 0
[size] => 0
)

)

seems to be the same (and correct) to me.. But the HTTP_Upload gives this:
(php code:
require_once 'HTTP/Upload.php';
$upload = new HTTP_Upload('en');
$file = $upload->getFiles('f');
if ( !$file->isError() )
print_r( $file );
end code)

http_upload_file Object
(
[_seeded] => 0
[upload] => Array
(
[real] => ' evil!!.torrent
[name] => ' evil!!.torrent
[form_name] => f
[ext] =>
[tmp_name] => C:\WINDOWS\TEMP\php9.tmp
[size] => 0
[type] => application/octet-stream
[error] => NO_USER_FILE
)

[mode_name_selected] =>
[_extensions_check] => Array
(
[0] => php
[1] => phtm
[2] => phtml
[3] => php3
[4] => inc
)

[_extensions_mode] => deny
[_chmod] => 432
[lang] => en
[html] =>
[_debug] =>
[_default_error_mode] =>
[_default_error_options] =>
[_default_error_handler] =>
[_error_class] => PEAR_Error
[_expected_errors] => Array
(
)

[error_codes] => Array
............

[2004-09-17 20:38 UTC] znoob2 at yahoo dot com

Now got the Upload.php file from
http://www.inf.ufsc.br/~antonio/pear/Upload.php
(17th of September)
It is a little better (the backslash is now added), but still "BI" is missing.
Thanks for all the effort and I hope we can find out what's wrong. Does it maybe matter where php runs (C:\Program Files\php)? Or on which drive my php file is (D:)? Or where PEAR runs (C:\Program Files\php\PEAR)?

[upload] => Array
(
[real] => \' evil!!.torrent
[name] => \' evil!!.torrent
[form_name] => f
[ext] =>
[tmp_name] => C:\WINDOWS\TEMP\phpB1.tmp
[size] => 0
[type] => application/octet-stream
[error] => NO_USER_FILE
)

[2004-09-19 21:33 UTC] znoob2 at yahoo dot com

Yes! That zero should do the trick. You're good.

0 -> BI\' evil!!.torrent
1 -> ' evil!!.torrent
2 -> "' evil!!.torrent"
3 -> ' evil!!.torrent
http_upload_file Object
(
[_seeded] => 0
[upload] => Array
(
[real] => \' evil!!.torrent
[name] => \' evil!!.torrent
[form_name] => f
[ext] =>
[tmp_name] => C:\WINDOWS\TEMP\php10.tmp
[size] => 0
[type] => application/octet-stream
[error] => NO_USER_FILE
)

[2004-09-23 18:42 UTC] znoob2 at yahoo dot com

Things are getting worse again... What seems to be the problem?
Thx,
Paul

0 -> ' evil!!.torrent
1 -> ' evil!!.torrenthttp_upload_file Object
(
[_seeded] => 0
[upload] => Array
(
[real] => \' evil!!.torrent
[name] => \' evil!!.torrent
[form_name] => f

[2004-10-07 14:30 UTC] znoob2 at yahoo dot com

Ah, I think I am starting to understand the problem. What happens is that somewhere the ' is escaped to \'. However in windows basename() takes the last index of \ and returns what follows it.
Why is the ' escaped?
That's because of the nasty magic quotes. So actually it's a magic quotes problem.
Would this be a solution? -> check magic quotes, then decide to stripslashes or not.
I haven't looked in your code, so could you - hopefully for the last time - update your code? then I download it to test again.

Thx

[2004-10-07 14:46 UTC] znoob2 at yahoo dot com

OK, what I did:
I think you don't need that basename() because a file that is sent, is never sent with the full path. Only the filename. So I removed all basename() function calls. Then line 455 I changed to:
$name = get_magic_quotes_gpc() ? stripslashes( $value['name'] ) : $value['name'];
Now the output seems to be completely correct.

One open question remains:
Is my claim true that it's safe to omit the basename call? If there's some sneeky hacker around modifying evil post data so that it DOES send a complete path, is it then possible for him / her to do malicious things?

[2004-10-07 14:54 UTC] znoob2 at yahoo dot com

Hm, still not everything compatible.

Must
if (DIRECTORY_SEPARATOR == '\\') {
$name = addslashes($name);
}
( i got it on line 584 )
not be:
if (DIRECTORY_SEPARATOR == '\\') {
$name = str_replace( '\', '\\', $name );
}

??? Actually, is it really needed?

[2004-10-07 15:08 UTC] znoob2 at yahoo dot com

Sorry, I was a little too enthousiastic so I posted a lot, but now my searching is complete. I tested it both with magic quotes on and off. And in all cases I got it returning the same value as $_FILES does.

line 439: $name = $value['name'][$key];
line 455: $name = $value['name'];
line 584: completely removed this code:
if (DIRECTORY_SEPARATOR == '\\' ) {
$name = addslashes( $name );
}

and I believe there was some call to basename before line 439. I removed it. I removed all calls to basename. So that's about it. It works now and I hope I didn't remove some important security checking.

Greetz

[2004-12-01 13:29 UTC] znoob2 at yahoo dot com

Sorry to tell you, but I recently bought a new computer and didn't backup the pear files. And besides that I wouldn't have a clue on how to make a diff.

A final remark: I'm not sure if the changes I make alter the security aspects of the class. Because I removed, as I remember, some checking and escaping or so. I believed those were redundant in my situation but I am not sure if they were redundant on every system and situation. For example, I am using windows and I don't know how I changed threads for Linux systems. So I think this change is not worth putting as an update in the public version as yet. Better other people that do know about Linux etc. take a look and a deep thought about it, then do the update.

I am willing to test any changes though, so please let me know about any progress.

Yours sincerely