MFH: improve variable name checks

add more tests

diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
index 735ffa27c4d5cb33f1313c1f1299add7b98c180b..765305e063e6eb84944bef444a588edb7cc0facb 100644 (file)
--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -6261,51 +6261,25 @@ static int copy_request_variable(void *pDest, int num_args, va_list args, zend_h
        prefix = va_arg(args, char *);
        prefix_len = va_arg(args, uint);
 
-       if (!prefix_len) {
-               if (!hash_key->nKeyLength) {
-                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Numeric key detected - possible security hazard.");
-                       return 0;
-               } else if (!strcmp(hash_key->arKey, "GLOBALS")) {
-                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted GLOBALS variable overwrite.");
-                       return 0; 
-               } else if (*hash_key->arKey == '_' && 
-                               (
-                                       !strcmp(hash_key->arKey, "_GET") || 
-                                       !strcmp(hash_key->arKey, "_POST") || 
-                                       !strcmp(hash_key->arKey, "_COOKIE") || 
-                                       !strcmp(hash_key->arKey, "_ENV") || 
-                                       !strcmp(hash_key->arKey, "_SERVER") || 
-                                       !strcmp(hash_key->arKey, "_SESSION") || 
-                                       !strcmp(hash_key->arKey, "_FILES") || 
-                                       !strcmp(hash_key->arKey, "_REQUEST")
-                               )
-                       ) {
-                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted super-global (%s) variable overwrite.", hash_key->arKey);
-                       return 0;       
-               } else if (*hash_key->arKey == 'H' && 
-                               (
-                                       !strcmp(hash_key->arKey, "HTTP_POST_VARS") || 
-                                       !strcmp(hash_key->arKey, "HTTP_GET_VARS") || 
-                                       !strcmp(hash_key->arKey, "HTTP_COOKIE_VARS") || 
-                                       !strcmp(hash_key->arKey, "HTTP_ENV_VARS") || 
-                                       !strcmp(hash_key->arKey, "HTTP_SERVER_VARS") || 
-                                       !strcmp(hash_key->arKey, "HTTP_RAW_POST_DATA") || 
-                                       !strcmp(hash_key->arKey, "HTTP_POST_FILES")
-                               )
-                       ) {
-                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted long input array (%s) overwrite.", hash_key->arKey);
-                       return 0;       
-               }
+       if (!prefix_len && !hash_key->nKeyLength) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "Numeric key detected - possible security hazard.");
+               return 0;
        }
 
        if (hash_key->nKeyLength) {
                new_key_len = prefix_len + hash_key->nKeyLength;
-               new_key = (char *) emalloc(new_key_len);
+               new_key = (char *) emalloc(new_key_len); /* +1 comes from nKeyLength */
 
                memcpy(new_key, prefix, prefix_len);
                memcpy(new_key+prefix_len, hash_key->arKey, hash_key->nKeyLength);
        } else {
                new_key_len = spprintf(&new_key, 0, "%s%ld", prefix, hash_key->h);
+               new_key_len++;
+       }
+
+       if (php_varname_check(new_key, new_key_len, 0 TSRMLS_CC) == FAILURE) {
+               efree(new_key);
+               return 0;
        }
 
        zend_delete_global_variable(new_key, new_key_len-1 TSRMLS_CC);

diff --git a/ext/standard/php_var.h b/ext/standard/php_var.h
index 1b6f180366c5f40dd7f1bd69f6b6cf5fe0ea38b1..6043f4317c0c7c13a147ac4760c66a19fc6ddd92 100644 (file)
--- a/ext/standard/php_var.h
+++ b/ext/standard/php_var.h
@@ -67,4 +67,48 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hash);
        
 PHPAPI zend_class_entry *php_create_empty_class(char *class_name, int len);
 
+static inline int php_varname_check(char *name, int name_len, zend_bool silent TSRMLS_DC) /* {{{ */
+{
+    if (name_len == sizeof("GLOBALS") && !memcmp(name, "GLOBALS", sizeof("GLOBALS"))) {
+               if (!silent) {
+                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted GLOBALS variable overwrite");
+               }
+        return FAILURE;
+    } else if (name[0] == '_' &&
+            (
+             (name_len == sizeof("_GET") && !memcmp(name, "_GET", sizeof("_GET"))) ||
+             (name_len == sizeof("_POST") && !memcmp(name, "_POST", sizeof("_POST"))) ||
+             (name_len == sizeof("_COOKIE") && !memcmp(name, "_COOKIE", sizeof("_COOKIE"))) ||
+             (name_len == sizeof("_ENV") && !memcmp(name, "_ENV", sizeof("_ENV"))) ||
+             (name_len == sizeof("_SERVER") && !memcmp(name, "_SERVER", sizeof("_SERVER"))) ||
+             (name_len == sizeof("_SESSION") && !memcmp(name, "_SESSION", sizeof("_SESSION"))) ||
+             (name_len == sizeof("_FILES") && !memcmp(name, "_FILES", sizeof("_FILES"))) ||
+             (name_len == sizeof("_REQUEST") && !memcmp(name, "_REQUEST", sizeof("_REQUEST")))
+            )
+            ) {
+               if (!silent) {
+                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted super-global (%s) variable overwrite", name);
+               }
+        return FAILURE;
+    } else if (name[0] == 'H' &&
+            (
+             (name_len == sizeof("HTTP_POST_VARS") && !memcmp(name, "HTTP_POST_VARS", sizeof("HTTP_POST_VARS"))) ||
+             (name_len == sizeof("HTTP_GET_VARS") && !memcmp(name, "HTTP_GET_VARS", sizeof("HTTP_GET_VARS"))) ||
+             (name_len == sizeof("HTTP_COOKIE_VARS") && !memcmp(name, "HTTP_COOKIE_VARS", sizeof("HTTP_COOKIE_VARS"))) ||
+             (name_len == sizeof("HTTP_ENV_VARS") && !memcmp(name, "HTTP_ENV_VARS", sizeof("HTTP_ENV_VARS"))) ||
+             (name_len == sizeof("HTTP_SERVER_VARS") && !memcmp(name, "HTTP_SERVER_VARS", sizeof("HTTP_SERVER_VARS"))) ||
+             (name_len == sizeof("HTTP_SESSION_VARS") && !memcmp(name, "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"))) ||
+             (name_len == sizeof("HTTP_RAW_POST_DATA") && !memcmp(name, "HTTP_RAW_POST_DATA", sizeof("HTTP_RAW_POST_DATA"))) ||
+             (name_len == sizeof("HTTP_POST_FILES") && !memcmp(name, "HTTP_POST_FILES", sizeof("HTTP_POST_FILES")))
+            )
+            ) {
+               if (!silent) {
+                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Attempted long input array (%s) overwrite", name);
+               }
+        return FAILURE;
+    }
+       return SUCCESS;
+}
+/* }}} */
+
 #endif /* PHP_VAR_H */

diff --git a/ext/standard/tests/general_functions/import_request1.phpt b/ext/standard/tests/general_functions/import_request1.phpt
new file mode 100644 (file)
index 0000000..cb6df69
--- /dev/null
+++ b/ext/standard/tests/general_functions/import_request1.phpt
@@ -0,0 +1,99 @@
+--TEST--
+import_request_variables() test (overwrite super-globals)
+--GET--
+GET=0&POST=1&COOKIE=2&FILES=3&REQUEST=4
+--POST--
+GET=5&POST=6&COOKIE=7&FILES=8&REQUEST=9
+--COOKIE--
+GET=10;POST=11;COOKIE=12;FILES=13;REQUEST=14
+--INI--
+variables_order=CGP
+--FILE--
+<?php
+
+import_request_variables("gpc", "_");
+var_dump($_GET, $_POST, $_COOKIE, $_FILES, $_REQUEST);
+
+echo "Done\n";
+?>
+--EXPECTF--
+Warning: import_request_variables(): Attempted super-global (_GET) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_POST) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_COOKIE) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_FILES) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_REQUEST) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_GET) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_POST) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_COOKIE) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_FILES) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_REQUEST) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_GET) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_POST) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_COOKIE) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_FILES) variable overwrite in %s on line %d
+
+Warning: import_request_variables(): Attempted super-global (_REQUEST) variable overwrite in %s on line %d
+array(5) {
+  ["GET"]=>
+  string(1) "0"
+  ["POST"]=>
+  string(1) "1"
+  ["COOKIE"]=>
+  string(1) "2"
+  ["FILES"]=>
+  string(1) "3"
+  ["REQUEST"]=>
+  string(1) "4"
+}
+array(5) {
+  ["GET"]=>
+  string(1) "5"
+  ["POST"]=>
+  string(1) "6"
+  ["COOKIE"]=>
+  string(1) "7"
+  ["FILES"]=>
+  string(1) "8"
+  ["REQUEST"]=>
+  string(1) "9"
+}
+array(5) {
+  ["GET"]=>
+  string(2) "10"
+  ["POST"]=>
+  string(2) "11"
+  ["COOKIE"]=>
+  string(2) "12"
+  ["FILES"]=>
+  string(2) "13"
+  ["REQUEST"]=>
+  string(2) "14"
+}
+array(0) {
+}
+array(5) {
+  ["GET"]=>
+  string(1) "5"
+  ["POST"]=>
+  string(1) "6"
+  ["COOKIE"]=>
+  string(1) "7"
+  ["FILES"]=>
+  string(1) "8"
+  ["REQUEST"]=>
+  string(1) "9"
+}
+Done

diff --git a/ext/standard/tests/general_functions/import_request2.phpt b/ext/standard/tests/general_functions/import_request2.phpt
new file mode 100644 (file)
index 0000000..eb27821
--- /dev/null
+++ b/ext/standard/tests/general_functions/import_request2.phpt
@@ -0,0 +1,25 @@
+--TEST--
+import_request_variables() test (numeric keys)
+--GET--
+1=0&2=1&3=2&4=3&5=4
+--POST--
+1=5&2=6&3=7&4=8&5=9
+--COOKIE--
+1=10;2=11;3=12;4=13;5=14
+--INI--
+variables_order=CGP
+--FILE--
+<?php
+
+import_request_variables("gpc", "_");
+var_dump($_1, $_2, $_3, $_4, $_5);
+
+echo "Done\n";
+?>
+--EXPECTF--
+string(2) "10"
+string(2) "11"
+string(2) "12"
+string(2) "13"
+string(2) "14"
+Done

diff --git a/ext/standard/tests/general_functions/import_request3.phpt b/ext/standard/tests/general_functions/import_request3.phpt
new file mode 100644 (file)
index 0000000..a9fba26
--- /dev/null
+++ b/ext/standard/tests/general_functions/import_request3.phpt
@@ -0,0 +1,25 @@
+--TEST--
+import_request_variables() test (numeric keys, different order)
+--GET--
+1=0&2=1&3=2&4=3&5=4
+--POST--
+1=5&2=6&3=7&4=8&5=9
+--COOKIE--
+1=10;2=11;3=12;4=13;5=14
+--INI--
+variables_order=CGP
+--FILE--
+<?php
+
+import_request_variables("gcp", "_");
+var_dump($_1, $_2, $_3, $_4, $_5);
+
+echo "Done\n";
+?>
+--EXPECTF--
+string(1) "5"
+string(1) "6"
+string(1) "7"
+string(1) "8"
+string(1) "9"
+Done