Skip to content
Permalink
Browse files Browse the repository at this point in the history
Use req::strndup in php_mb_parse_encoding_list to prevent oob memory …
…write.

Summary:
Fix out of bounds write access in mb_detect_encoding.  Using strndup in
php_mb_parse_encoding_list will cause strings with embedded nulls to be
unexpectedly shortened.  The expected length of the string is tracked in
value_length but since strndup may copy fewer characters when there are
mbedded null this can lead to oob writes into tmpstr.

I've found a couple other places in this file that use strndup and replaced
them with req::strndup as well.  The use of strndup in mb_send_mail also seemed
to be a leak.

This replaces uses of strndup with req::strndup which can handle embedded
nulls properly.

It looks like I also accidentally fixed t11337047 at the same time.  Adding it to the list of tasks.

Reviewed By: paulbiss

Differential Revision: D3360065

fbshipit-source-id: 99776cf9105e3789883380bf30240009eec52cec
  • Loading branch information
Bill Nell authored and Hhvm Bot committed Jul 1, 2016
1 parent 2f6e219 commit 365abe8
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
17 changes: 10 additions & 7 deletions hphp/runtime/ext/mbstring/ext_mbstring.cpp
Expand Up @@ -554,11 +554,11 @@ static int php_mb_parse_encoding_list(const char *value, int value_length,

/* copy the value string for work */
if (value[0]=='"' && value[value_length-1]=='"' && value_length>2) {
tmpstr = (char *)strndup(value+1, value_length-2);
tmpstr = req::strndup(value + 1, value_length - 2);
value_length -= 2;
} else {
tmpstr = req::strndup(value, value_length);
}
else
tmpstr = (char *)strndup(value, value_length);
if (tmpstr == nullptr) {
return 0;
}
Expand Down Expand Up @@ -640,7 +640,7 @@ static int php_mb_parse_encoding_list(const char *value, int value_length,
}
ret = 0;
}
free(tmpstr);
req::free(tmpstr);
}

return ret;
Expand Down Expand Up @@ -2254,11 +2254,11 @@ bool HHVM_FUNCTION(mb_parse_str,
info.num_from_encodings = MBSTRG(http_input_list_size);
info.from_language = MBSTRG(current_language);

char *encstr = strndup(encoded_string.data(), encoded_string.size());
char *encstr = req::strndup(encoded_string.data(), encoded_string.size());
Array resultArr = Array::Create();
mbfl_encoding *detected =
_php_mb_encoding_handler_ex(&info, resultArr, encstr);
free(encstr);
req::free(encstr);
result.assignIfRef(resultArr);

MBSTRG(http_input_identify) = detected;
Expand Down Expand Up @@ -4251,7 +4251,7 @@ bool HHVM_FUNCTION(mb_send_mail,
if (!to.empty()) {
int to_len = to.size();
if (to_len > 0) {
to_r = strndup(to.data(), to_len);
to_r = req::strndup(to.data(), to_len);
for (; to_len; to_len--) {
if (!isspace((unsigned char)to_r[to_len - 1])) {
break;
Expand Down Expand Up @@ -4398,6 +4398,9 @@ bool HHVM_FUNCTION(mb_send_mail,
encoded_message.data(),
all_headers, cmd.data()));
mbfl_memory_device_clear(&device);
if (to_r != to.data()) {
req::free(to_r);
}
return ret;
}

Expand Down
6 changes: 6 additions & 0 deletions hphp/test/quick/mbstring-oob.php
@@ -0,0 +1,6 @@
<?hh // strict
// Copyright 2004-present Facebook. All Rights Reserved.

$var0 = null;
$var3 = "\x00";
$var3 = mb_detect_encoding($var0,$var3);

0 comments on commit 365abe8

Please sign in to comment.