Vulnerability: Multipart-file uploads call variables to be improperly registered in the global scope. In cases where variables are not declared explicitly before being used this can lead to unexpected behavior. This affects all supported versions of HHVM prior to the patch (3.25.1, 3.24.5, and 3.21.9 and below).
Vulnerability ID: CVE-2018-6334
CWE: CWE-20
Severity score: 6.4
Project: https://github.com/facebook/hhvm
Published: 2018-12-31
File: hphp/runtime/server/upload.cpp
Commit message: [security][CVE-2018-6334] kill globals for file uploads in hhvm

Code should look at _GET, _POST, _FILES, etc. directly to avoid security concerns.

This included both `$GLOBALS` and `register_globals`-like behavior.

CVE-2018-6334
Commit author: Fred Emmott

Vulnerable version (before fix):
 */  #include "hphp/runtime/server/upload.h" #include "hphp/runtime/base/program-functions.h" #include "hphp/runtime/base/runtime-option.h" #include "hphp/runtime/base/request-local.h" #include "hphp/runtime/base/zend-printf.h" #include "hphp/runtime/base/php-globals.h" #include "hphp/runtime/ext/apc/ext_apc.h" #include "hphp/util/logger.h" #include "hphp/runtime/base/string-util.h" #include "hphp/util/text-util.h" #include "hphp/runtime/base/request-event-handler.h" #include <folly/FileUtil.h>  using std::set; void rfc1867PostHandler(Transport* transport,   std::string array_index, abuf;   char *lbuf=nullptr;   int total_bytes=0, cancel_upload=0, is_arr_upload=0, array_len=0;   int max_file_size=0, skip_upload=0, anonindex=0, is_anonymous;   std::set<std::string> &uploaded_files = s_rfc1867_data->rfc1867UploadedFiles;   multipart_buffer *mbuff;   int fd=-1; void rfc1867PostHandler(Transport* transport,       }        if (!param) {         is_anonymous = 1;         param = (char*)malloc(MAX_SIZE_ANONNAME);         snprintf(param, MAX_SIZE_ANONNAME, "%u", anonindex++);       } else {         is_anonymous = 0;       }        /* New Rule: never repair potential malicious user input */ void rfc1867PostHandler(Transport* transport,         s = tmp;       }        Array globals = php_globals_as_array();       if (!is_anonymous) {         if (s) {           String val(s+1, strlen(s+1), CopyString);           safe_php_register_variable(lbuf, val, globals, 0);         } else {           String val(filename, strlen(filename), CopyString);           safe_php_register_variable(lbuf, val, globals, 0);         }       }        /* Add $foo[name] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[name][%s]", void rfc1867PostHandler(Transport* transport,         }       }        /* Add $foo_type */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s_type[%s]",                  abuf.c_str(), array_index.c_str());       } else {         snprintf(lbuf, llen, "%s_type", param);       }       if (!is_anonymous) {         String val(cd, strlen(cd), CopyString);         safe_php_register_variable(lbuf, val, globals, 0);       }        /* Add $foo[type] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[type][%s]", void rfc1867PostHandler(Transport* transport,        Variant tempFileName(temp_filename);        /* if param is of form xxx[.*] this will cut it to xxx */       if (!is_anonymous) {         safe_php_register_variable(param, tempFileName, globals, 1);       }        /* Add $foo[tmp_name] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[tmp_name][%s]", void rfc1867PostHandler(Transport* transport,       }       safe_php_register_variable(lbuf, error_type, files, 0);        /* Add $foo_size */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s_size[%s]",                  abuf.c_str(), array_index.c_str());       } else {         snprintf(lbuf, llen, "%s_size", param);       }       if (!is_anonymous) {         safe_php_register_variable(lbuf, file_size, globals, 0);       }        /* Add $foo[size] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[size][%s]",

Fixed version (after fix):
 */  #include "hphp/runtime/server/upload.h"  #include "hphp/runtime/base/program-functions.h" #include "hphp/runtime/base/request-event-handler.h" #include "hphp/runtime/base/request-local.h" #include "hphp/runtime/base/runtime-option.h" #include "hphp/runtime/base/string-util.h" #include "hphp/runtime/base/zend-printf.h" #include "hphp/runtime/ext/apc/ext_apc.h"  #include "hphp/util/logger.h" #include "hphp/util/text-util.h"  #include <folly/FileUtil.h>  using std::set; void rfc1867PostHandler(Transport* transport,   std::string array_index, abuf;   char *lbuf=nullptr;   int total_bytes=0, cancel_upload=0, is_arr_upload=0, array_len=0;   int max_file_size=0, skip_upload=0, anonindex=0;   std::set<std::string> &uploaded_files = s_rfc1867_data->rfc1867UploadedFiles;   multipart_buffer *mbuff;   int fd=-1; void rfc1867PostHandler(Transport* transport,       }        if (!param) {         param = (char*)malloc(MAX_SIZE_ANONNAME);         snprintf(param, MAX_SIZE_ANONNAME, "%u", anonindex++);       }        /* New Rule: never repair potential malicious user input */ void rfc1867PostHandler(Transport* transport,         s = tmp;       }        /* Add $foo[name] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[name][%s]", void rfc1867PostHandler(Transport* transport,         }       }        /* Add $foo[type] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[type][%s]", void rfc1867PostHandler(Transport* transport,        Variant tempFileName(temp_filename);        /* Add $foo[tmp_name] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[tmp_name][%s]", void rfc1867PostHandler(Transport* transport,       }       safe_php_register_variable(lbuf, error_type, files, 0);        /* Add $foo[size] */       if (is_arr_upload) {         snprintf(lbuf, llen, "%s[size][%s]",

Patch:
patch: @@ -16,16 +16,18 @@
 */
 
 #include "hphp/runtime/server/upload.h"
+
 #include "hphp/runtime/base/program-functions.h"
-#include "hphp/runtime/base/runtime-option.h"
+#include "hphp/runtime/base/request-event-handler.h"
 #include "hphp/runtime/base/request-local.h"
+#include "hphp/runtime/base/runtime-option.h"
+#include "hphp/runtime/base/string-util.h"
 #include "hphp/runtime/base/zend-printf.h"
-#include "hphp/runtime/base/php-globals.h"
 #include "hphp/runtime/ext/apc/ext_apc.h"
+
 #include "hphp/util/logger.h"
-#include "hphp/runtime/base/string-util.h"
 #include "hphp/util/text-util.h"
-#include "hphp/runtime/base/request-event-handler.h"
+
 #include <folly/FileUtil.h>
 
 using std::set;
@@ -714,7 +716,7 @@ void rfc1867PostHandler(Transport* transport,
   std::string array_index, abuf;
   char *lbuf=nullptr;
   int total_bytes=0, cancel_upload=0, is_arr_upload=0, array_len=0;
-  int max_file_size=0, skip_upload=0, anonindex=0, is_anonymous;
+  int max_file_size=0, skip_upload=0, anonindex=0;
   std::set<std::string> &uploaded_files = s_rfc1867_data->rfc1867UploadedFiles;
   multipart_buffer *mbuff;
   int fd=-1;
@@ -855,11 +857,8 @@ void rfc1867PostHandler(Transport* transport,
       }
 
       if (!param) {
-        is_anonymous = 1;
         param = (char*)malloc(MAX_SIZE_ANONNAME);
         snprintf(param, MAX_SIZE_ANONNAME, "%u", anonindex++);
-      } else {
-        is_anonymous = 0;
       }
 
       /* New Rule: never repair potential malicious user input */
@@ -1067,17 +1066,6 @@ void rfc1867PostHandler(Transport* transport,
         s = tmp;
       }
 
-      Array globals = php_globals_as_array();
-      if (!is_anonymous) {
-        if (s) {
-          String val(s+1, strlen(s+1), CopyString);
-          safe_php_register_variable(lbuf, val, globals, 0);
-        } else {
-          String val(filename, strlen(filename), CopyString);
-          safe_php_register_variable(lbuf, val, globals, 0);
-        }
-      }
-
       /* Add $foo[name] */
       if (is_arr_upload) {
         snprintf(lbuf, llen, "%s[name][%s]",
@@ -1107,18 +1095,6 @@ void rfc1867PostHandler(Transport* transport,
         }
       }
 
-      /* Add $foo_type */
-      if (is_arr_upload) {
-        snprintf(lbuf, llen, "%s_type[%s]",
-                 abuf.c_str(), array_index.c_str());
-      } else {
-        snprintf(lbuf, llen, "%s_type", param);
-      }
-      if (!is_anonymous) {
-        String val(cd, strlen(cd), CopyString);
-        safe_php_register_variable(lbuf, val, globals, 0);
-      }
-
       /* Add $foo[type] */
       if (is_arr_upload) {
         snprintf(lbuf, llen, "%s[type][%s]",
@@ -1140,11 +1116,6 @@ void rfc1867PostHandler(Transport* transport,
 
       Variant tempFileName(temp_filename);
 
-      /* if param is of form xxx[.*] this will cut it to xxx */
-      if (!is_anonymous) {
-        safe_php_register_variable(param, tempFileName, globals, 1);
-      }
-
       /* Add $foo[tmp_name] */
       if (is_arr_upload) {
         snprintf(lbuf, llen, "%s[tmp_name][%s]",
@@ -1174,17 +1145,6 @@ void rfc1867PostHandler(Transport* transport,
       }
       safe_php_register_variable(lbuf, error_type, files, 0);
 
-      /* Add $foo_size */
-      if (is_arr_upload) {
-        snprintf(lbuf, llen, "%s_size[%s]",
-                 abuf.c_str(), array_index.c_str());
-      } else {
-        snprintf(lbuf, llen, "%s_size", param);
-      }
-      if (!is_anonymous) {
-        safe_php_register_variable(lbuf, file_size, globals, 0);
-      }
-
       /* Add $foo[size] */
       if (is_arr_upload) {
         snprintf(lbuf, llen, "%s[size][%s]",
path: hphp/runtime/server/upload.cpp