Re: [PHP-CVS] cvs: php-src(PHP_4_3) / NEWS /ext/standard string.c /main rfc1867.c

Fri, 21 Jan 2005 07:05:18 -0800

Uhm yes. The purpose of the code is to trim characters from the filename that could be interpreted as a path consequently leading to all sorts of interesting vulnerabilities. Given that we have a function for this very purpose called php_basename() it would only seem logic to use it, instead of duplicating it's functionality (incorrectly I might add).

Ilia


Moriyoshi Koizumi wrote: 
On 2005/01/21, at 2:57, Ilia Alshanetsky wrote:

iliaa        Thu Jan 20 12:57:42 2005 EDT

Modified files: (Branch: PHP_4_3)
/php-src NEWS
/php-src/main rfc1867.c
/php-src/ext/standard string.c
Log:
MFH: Fixed bug #31398 (When magic_guotes_gpc are enabled filenames with '
get cutoff).


http://cvs.php.net/diff.php/php-src/NEWS? r1=1.1247.2.810&r2=1.1247.2.811&ty=u
Index: php-src/NEWS
diff -u php-src/NEWS:1.1247.2.810 php-src/NEWS:1.1247.2.811
--- php-src/NEWS:1.1247.2.810 Wed Jan 19 20:43:19 2005
+++ php-src/NEWS Thu Jan 20 12:57:40 2005
@@ -19,6 +19,8 @@
- Fixed bug #31174 (compile warning in url.c). (Ilia, lukem at NetBSD dot org)
- Fixed bug #31159 (COM object access is not working). (Wez)
- Fixed bug #31142 (imap_mail_compose() fails to generate correct output). (Ilia)
+- Fixed bug #31398 (When magic_guotes_gpc are enabled filenames with ' get cutoff).
+ (Ilia)
- Fixed bug #31120 (mssql_query returns false on successfull inserts and
stored procedures). (Frank)
- Fixed bugs #31107, #31110, #31111 (Compile failure of zend_strtod.c). (Jani)
http://cvs.php.net/diff.php/php-src/main/rfc1867.c? r1=1.122.2.28&r2=1.122.2.29&ty=u
Index: php-src/main/rfc1867.c
diff -u php-src/main/rfc1867.c:1.122.2.28 php-src/main/rfc1867.c:1.122.2.29
--- php-src/main/rfc1867.c:1.122.2.28 Sat Nov 20 15:16:44 2004
+++ php-src/main/rfc1867.c Thu Jan 20 12:57:41 2005
@@ -16,7 +16,7 @@
| Jani Taskinen <[EMAIL PROTECTED]> |
+---------------------------------------------------------------------- +
*/
-/* $Id: rfc1867.c,v 1.122.2.28 2004/11/20 20:16:44 sesser Exp $ */
+/* $Id: rfc1867.c,v 1.122.2.29 2005/01/20 17:57:41 iliaa Exp $ */

 /*
  *  This product includes software developed by the Apache Group
@@ -31,6 +31,7 @@
 #include "php_globals.h"
 #include "php_variables.h"
 #include "rfc1867.h"
+#include "ext/standard/php_string.h"

 #undef DEBUG_FILE_UPLOAD

@@ -842,7 +843,7 @@
     while (!multipart_buffer_eof(mbuff TSRMLS_CC))
     {
         char buff[FILLUNIT];
-        char *cd=NULL,*param=NULL,*filename=NULL, *tmp=NULL;
+        char *cd=NULL,*param=NULL,*filename=NULL;
         int blen=0, wlen=0;

zend_llist_clean(&header);
@@ -1064,30 +1065,13 @@
str_len = strlen(filename);
php_mb_gpc_encoding_converter(&filename, &str_len, 1, NULL, NULL TSRMLS_CC);
}
- s = php_mb_strrchr(filename, '\\' TSRMLS_CC);
- if ((tmp = php_mb_strrchr(filename, '/' TSRMLS_CC)) > s) {
- s = tmp;
- }
num_vars--;
- } else {
- s = strrchr(filename, '\\');
- if ((tmp = strrchr(filename, '/')) > s) {
- s = tmp;
- }
- }
-#else
- s = strrchr(filename, '\\');
- if ((tmp = strrchr(filename, '/')) > s) {
- s = tmp;
}
#endif
- if (PG(magic_quotes_gpc)) {
- s = s ? s : filename;
- tmp = strrchr(s, '\'');
- s = tmp > s ? tmp : s;
- tmp = strrchr(s, '"');
- s = tmp > s ? tmp : s;
- }
+ /* ensure that the uploaded file name only contains the path */
+ s = php_basename(filename, strlen(filename), NULL, 0);
+ efree(filename);
+ filename = s; 


Are you really sure how the removed part of code would work?
Obviously you broke something. Please clarify.

Moriyoshi




--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to