| Package home | Report new bug | New search | Development Roadmap | Status: Open | Feedback | All | Closed Since Version 1.0.0b4 |
| Bug #2253 | quotes | ||
|---|---|---|---|
| Submitted: | 2004-08-30 15:46 UTC | ||
| From: | znoob2 at yahoo dot com | Assigned: | |
| Status: | No Feedback | Package: | HTTP_Upload |
| PHP Version: | 4.3.6 | OS: | WinXP |
| Roadmaps: | (Not assigned) | ||
[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
...
*/
Comments
[2004-08-30 15:58 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[2004-08-30 18:25 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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:46 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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-08 17:52 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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-17 21:04 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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-20 11:53 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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-09-27 12:45 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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-11-25 16:28 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!
[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
[2004-12-02 12:50 UTC] User who submitted this comment has not confirmed identity
If you submitted this note, check your email.If you do not have a message, click here to re-send MANUAL CONFIRMATION IS NOT POSSIBLE. Write a message to pear-dev@lists.php.net to request the confirmation link. All bugs/comments/patches associated with this email address will be deleted within 48 hours if the account request is not confirmed!