Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix a bug in number_format()
Summary: In case of error while calling snprintf, return empty string instead of malformed string.

Reviewed By: binliu19

Differential Revision: D13379372

fbshipit-source-id: 1e5611598464daccc674d5a96558c9f2aee08fc8
  • Loading branch information
ottoni authored and hhvm-bot committed Dec 14, 2018
1 parent da1cc76 commit 190ffdf
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
2 changes: 2 additions & 0 deletions hphp/runtime/base/zend-string.cpp
Expand Up @@ -1768,6 +1768,7 @@ String string_number_format(double d, int dec,
String tmpstr(63, ReserveString);
tmpbuf = tmpstr.mutableData();
tmplen = snprintf(tmpbuf, 64, "%.*F", dec, d);
if (tmplen < 0) return empty_string();
if (tmpbuf == nullptr || !isdigit((int)tmpbuf[0])) {
tmpstr.setSize(tmplen);
return tmpstr;
Expand All @@ -1777,6 +1778,7 @@ String string_number_format(double d, int dec,
tmpstr = String(tmplen, ReserveString);
tmpbuf = tmpstr.mutableData();
tmplen = snprintf(tmpbuf, tmplen + 1, "%.*F", dec, d);
if (tmplen < 0) return empty_string();
if (tmpbuf == nullptr || !isdigit((int)tmpbuf[0])) {
tmpstr.setSize(tmplen);
return tmpstr;
Expand Down
19 changes: 19 additions & 0 deletions hphp/test/slow/string/number_format_error.php
@@ -0,0 +1,19 @@
<?php
$READ_LENGTH = 0x1000; // choose leak size
// construct fake iptc header for controlled read
$iptc_hdr =
"\x1c\x01" . // magic
"\x00\x80" . // dataset, recnum
"\x00" . // padding
pack("N", $READ_LENGTH);
// spray a bit so it's near the broken string
$holder = [];
for($i = 0; $i < 100; $i++)
$holder[] = str_pad($iptc_hdr, 96);
// trigger bug to create string with len=-1
$badstr = number_format(0,0x7fffffff);
var_dump($badstr);
// leak memory :)
$tmp = iptcparse($badstr);
var_dump($tmp);
?>
2 changes: 2 additions & 0 deletions hphp/test/slow/string/number_format_error.php.expect
@@ -0,0 +1,2 @@
string(0) ""
bool(false)

0 comments on commit 190ffdf

Please sign in to comment.