Skip to content
Permalink
Browse files Browse the repository at this point in the history
string_number_format: Correctly handles return value of snprintf
Summary: `snprintf` can return a value greater than the number of bytes copied. In case the first byte of the string is not a digit (could be '-'), size of `tmpstr` was being updated without checking `tmplen`. This resulted in either an assertion error or a heap overflow depending on whether the assertion is compiled or not.

Reviewed By: mofarrell, qianxuweiren

Differential Revision: D17327899

fbshipit-source-id: ee53875d21e02608c6d870388eecf1464de24ff1
  • Loading branch information
DhavalKapil authored and hhvm-bot committed Sep 18, 2019
1 parent ef76449 commit dbeb9a5
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 1 deletion.
6 changes: 5 additions & 1 deletion hphp/runtime/base/zend-string.cpp
Expand Up @@ -1618,11 +1618,15 @@ String string_number_format(double d, int dec,
d = php_math_round(d, dec);

// departure from PHP: we got rid of dependencies on spprintf() here.
// This actually means 63 bytes for characters + 1 byte for '\0'
String tmpstr(63, ReserveString);
tmpbuf = tmpstr.mutableData();
tmplen = snprintf(tmpbuf, 64, "%.*F", dec, d);
// From the man page of snprintf, the return value is:
// The number of characters that would have been written if n had been
// sufficiently large, not counting the terminating null character.
if (tmplen < 0) return empty_string();
if (tmpbuf == nullptr || !isdigit((int)tmpbuf[0])) {
if (tmplen < 64 && (tmpbuf == nullptr || !isdigit((int)tmpbuf[0]))) {
tmpstr.setSize(tmplen);
return tmpstr;
}
Expand Down
9 changes: 9 additions & 0 deletions hphp/test/slow/string/number_format_t53795309.php
@@ -0,0 +1,9 @@
<?hh
// Copyright 2004-present Facebook. All Rights Reserved.

<<__EntryPoint>>
function main() {
$bin_repr = "\x00\x00\x00\x00\x00\x00\x00\x80";
$double_num = unpack("dnum", $bin_repr)['num'];
var_dump(number_format($double_num, 100));
}
1 change: 1 addition & 0 deletions hphp/test/slow/string/number_format_t53795309.php.expect
@@ -0,0 +1 @@
string(103) "-0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"

0 comments on commit dbeb9a5

Please sign in to comment.