Skip to content
Permalink
Browse files

[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
  • Loading branch information...
fredemmott committed Mar 30, 2018
1 parent 305fa86 commit 6937de5544c3eead3466b75020d8382080ed0cff
Showing with 7 additions and 47 deletions.
  1. +7 −47 hphp/runtime/server/upload.cpp
@@ -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]",

0 comments on commit 6937de5

Please sign in to comment.
You can’t perform that action at this time.