Fwd: [PHP-CVS] cvs: php4 /main php_globals.h rfc1867.c

I've fixed the RFC1867 problem in a way that should not have any side effects. It should be impossible to overwrite any of the variables that contain uploaded file names. I didn't have time to test it, if any of you can give me some indication on whether it works or not, I'd appreciate it. Zeev

Delivered-To: alias-zend-zeev-php-cvs@zend.com Mailing-List: contact php-cvs-help@lists.php.net; run by ezmlm list-help: <mailto:php-cvs-help@lists.php.net> list-unsubscribe: <mailto:php-cvs-unsubscribe@lists.php.net> list-post: <mailto:php-general@lists.php.net> Delivered-To: mailing list php-cvs@lists.php.net From: "Zeev Suraski" <zeev@zend.com> To: php-cvs@lists.php.net Date: Mon, 04 Sep 2000 19:07:51 -0000 Subject: [PHP-CVS] cvs: php4 /main php_globals.h rfc1867.c

zeev            Mon Sep  4 12:07:51 2000 EDT

Modified files:

    /php4/main  php_globals.h rfc1867.c

Log: Fix the file upload security problem with no side effects (untested) Index: php4/main/php_globals.h diff -u php4/main/php_globals.h:1.53 php4/main/php_globals.h:1.54

--- php4/main/php_globals.h:1.53        Tue Jul  4 02:15:06 2000
+++ php4/main/php_globals.h     Mon Sep  4 12:07:50 2000

@@ -94,6 +94,8 @@

        char *gpc_order;
        char *variables_order;

+       HashTable rfc1867_protected_variables;

+

        short connection_status;
        short ignore_user_abort;

@@ -113,7 +115,7 @@

        zend_bool html_errors;

-       zend_bool modules_activated;
+       zend_bool modules_activated;

}; Index: php4/main/rfc1867.c diff -u php4/main/rfc1867.c:1.39 php4/main/rfc1867.c:1.40

--- php4/main/rfc1867.c:1.39    Sun Sep  3 22:09:46 2000

+++ php4/main/rfc1867.c Mon Sep 4 12:07:50 2000 @@ -15,7 +15,7 @@

    | Authors: Rasmus Lerdorf <rasmus@php.net>                             |
    +----------------------------------------------------------------------+

*/ -/* $Id: rfc1867.c,v 1.39 2000/09/04 05:09:46 rasmus Exp $ */ +/* $Id: rfc1867.c,v 1.40 2000/09/04 19:07:50 zeev Exp $ */ #include <stdio.h> #include "php.h" @@ -28,18 +28,47 @@ #define NEW_BOUNDARY_CHECK 1 -#define SAFE_RETURN { if (namebuf) efree(namebuf); if (filenamebuf) efree(filenamebuf); if (lbuf) efree(lbuf); if (abuf) efree(abuf); if(arr_index) efree(arr_index); return; } +#define SAFE_RETURN { if (namebuf) efree(namebuf); if (filenamebuf) efree(filenamebuf); if (lbuf) efree(lbuf); if (abuf) efree(abuf); if(arr_index) efree(arr_index); zend_hash_destroy(&PG(rfc1867_protected_variables)); return; } /* The longest property name we use in an uploaded file array */ #define MAX_SIZE_OF_INDEX sizeof("[tmp_name]") +static void add_protected_variable(char *varname PLS_DC) +{

+       int dummy=0;

+

+       zend_hash_add(&PG(rfc1867_protected_variables), varname, strlen(varname)+1, &dummy, sizeof(int), NULL);

+} + + +static zend_bool is_protected_variable(char *varname PLS_DC) +{

+       return zend_hash_exists(&PG(rfc1867_protected_variables), varname, strlen(varname)+1);

+} + + +static void safe_php_register_variable(char *var, char *strval, zval *track_vars_array ELS_DC PLS_DC) +{

+       if (!is_protected_variable(strval PLS_CC)) {
+               php_register_variable(var, strval, track_vars_array ELS_CC PLS_CC);
+       }

+} + +static void safe_php_register_variable_ex(char *var, zval *val, pval *track_vars_array ELS_DC PLS_DC) +{

+       if (!is_protected_variable(var PLS_CC)) {
+               php_register_variable_ex(var, val, track_vars_array ELS_CC PLS_CC);
+       }

+} + + static void register_http_post_files_variable(char *strvar, char *val, zval *http_post_files ELS_DC PLS_DC) {

        int register_globals = PG(register_globals);

- +

        PG(register_globals) = 0;
-       php_register_variable(strvar, val, http_post_files ELS_CC PLS_CC);
+       safe_php_register_variable(strvar, val, http_post_files ELS_CC PLS_CC);
        PG(register_globals) = register_globals;

} @@ -47,9 +76,9 @@ static void register_http_post_files_variable_ex(char *var, zval *val, zval *http_post_files ELS_DC PLS_DC) {

        int register_globals = PG(register_globals);

- +

        PG(register_globals) = 0;
-       php_register_variable_ex(var, val, http_post_files ELS_CC PLS_CC);
+       safe_php_register_variable_ex(var, val, http_post_files ELS_CC PLS_CC);
        PG(register_globals) = register_globals;

} @@ -64,13 +93,15 @@

        int eolsize;
        long bytes, max_file_size = 0;
        char *namebuf=NULL, *filenamebuf=NULL, *lbuf=NULL,
-               *abuf=NULL, *start_arr=NULL, *end_arr=NULL, *arr_index=NULL, *sbuf=NULL;
+               *abuf=NULL, *start_arr=NULL, *end_arr=NULL, *arr_index=NULL;
        FILE *fp;
        int itype, is_arr_upload=0, arr_len=0;
        zval *http_post_files=NULL;
        ELS_FETCH();
        PLS_FETCH();

+       zend_hash_init(&PG(rfc1867_protected_variables), 5, NULL, NULL, 0);

+

        if (PG(track_vars)) {
                ALLOC_ZVAL(http_post_files);
                array_init(http_post_files);

@@ -78,7 +109,6 @@

                PG(http_globals).post_files = http_post_files;
        }

-

        ptr = buf;
        rem = cnt;
        len = strlen(boundary);

@@ -172,16 +202,17 @@

                                                }
                                                abuf = estrndup(namebuf, strlen(namebuf)-arr_len);
                                                sprintf(lbuf, "%s_name[%s]", abuf, arr_index);
-                                               sbuf = estrdup(abuf);
+ add_protected_variable(lbuf PLS_CC);
+ add_protected_variable(abuf PLS_CC);
                                        } else {
                                                sprintf(lbuf, "%s_name", namebuf);
-                                               sbuf = estrdup(abuf);
+ add_protected_variable(abuf PLS_CC);
                                        }
                                        s = strrchr(filenamebuf, '\\');
                                        if (s && s > filenamebuf) {
- php_register_variable(lbuf, s+1, NULL ELS_CC PLS_CC);
+ safe_php_register_variable(lbuf, s+1, NULL ELS_CC PLS_CC);
                                        } else {
- php_register_variable(lbuf, filenamebuf, NULL ELS_CC PLS_CC);
+ safe_php_register_variable(lbuf, filenamebuf, NULL ELS_CC PLS_CC);
                                        }

                                        /* Add $foo[name] */

@@ -223,7 +254,7 @@

                                        } else {
                                                sprintf(lbuf, "%s_type", namebuf);
                                        }
-                                       php_register_variable(lbuf, s, NULL ELS_CC PLS_CC);
+                                       safe_php_register_variable(lbuf, s, NULL ELS_CC PLS_CC);

                                        /* Add $foo[type] */
                                        if (is_arr_upload) {

@@ -256,9 +287,7 @@

                                /* Check to make sure we are not overwriting special file
                                * upload variables */
-                               if(memcmp(namebuf,sbuf,strlen(sbuf))) {
-                                       php_register_variable(namebuf, ptr, array_ptr ELS_CC PLS_CC);
-                               }
+                               safe_php_register_variable(namebuf, ptr, array_ptr ELS_CC PLS_CC);

                                /* And a little kludge to pick out special MAX_FILE_SIZE */
                                itype = php_check_ident_type(namebuf);

@@ -322,7 +351,7 @@

                                                php_error(E_WARNING, "Only %d bytes were written, expected to write %ld", bytes, loc - ptr - 4);
                                        }
                                }
-                               php_register_variable(namebuf, fn, NULL ELS_CC PLS_CC);
+                               safe_php_register_variable(namebuf, fn, NULL ELS_CC PLS_CC);

                                /* Add $foo[tmp_name] */
                                if(is_arr_upload) {

@@ -343,7 +372,7 @@

                                        } else {
                                                sprintf(lbuf, "%s_size", namebuf);
                                        }
-                                       php_register_variable_ex(lbuf, &file_size, NULL ELS_CC PLS_CC);
+ safe_php_register_variable_ex(lbuf, &file_size, NULL ELS_CC PLS_CC);

                                        /* Add $foo[size] */
                                        if(is_arr_upload) {

@@ -359,7 +388,6 @@

                                break;
                }
        }
-       if(sbuf) efree(sbuf);
        SAFE_RETURN;

} -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, e-mail: php-cvs-unsubscribe@lists.php.net For additional commands, e-mail: php-cvs-help@lists.php.net To contact the list administrators, e-mail: php-list-admin@lists.php.net

-- Zeev Suraski <zeev@zend.com> http://www.zend.com/