Skip to content

Commit 6a20b55

Browse files
arnaud-lbarnaud-lb
committed
Handle overflows
1 parent c559710 commit 6a20b55

File tree

9 files changed

+182
-74
lines changed

9 files changed

+182
-74
lines changed

‎Zend/tests/zend_ini_parse_quantity_error.phpt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ foreach ($tests as $setting) {
1818
var_dump(zend_test_zend_ini_parse_quantity($setting));
1919
}
2020
--EXPECTF--
21-
Warning: Invalid quantity 'K': no valid leading digits, interpreting as '0' for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
21+
Warning: Invalid quantity "K": no valid leading digits, interpreting as "0" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
2222
int(0)
2323

24-
Warning: Invalid quantity '1KM', interpreting as '1M' for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
24+
Warning: Invalid quantity "1KM", interpreting as "1M" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
2525
int(1048576)
2626

27-
Warning: Invalid quantity '1X': unknown multipler 'X', interpreting as '1' for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
27+
Warning: Invalid quantity "1X": unknown multipler "X", interpreting as "1" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
2828
int(1)
2929

30-
Warning: Invalid quantity '1.0K', interpreting as '1K' for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
30+
Warning: Invalid quantity "1.0K", interpreting as "1K" for backwards compatibility in %s%ezend_ini_parse_quantity_error.php on line %d
3131
int(1024)

‎Zend/tests/zend_ini_parse_quantity_ini_set_error.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Test ini_set() with invalid quantity syntax
2+
Test ini_set() with invalid quantity
33
--EXTENSIONS--
44
zend_test
55
--FILE--
@@ -8,6 +8,6 @@ zend_test
88
var_dump(ini_set("zend_test.quantity_value", "1MB"));
99
var_dump(ini_get("zend_test.quantity_value"));
1010
--EXPECTF--
11-
Warning: Invalid "zend_test.quantity_value" setting: Invalid quantity '1MB': unknown multipler 'B', interpreting as '1' for backwards compatibility in %s on line %d
11+
Warning: Invalid "zend_test.quantity_value" setting: Invalid quantity "1MB": unknown multipler "B", interpreting as "1" for backwards compatibility in %s on line %d
1212
string(1) "0"
1313
string(3) "1MB"

‎Zend/tests/zend_ini_parse_quantity_null.phpt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,20 @@ var_dump(zend_test_zend_ini_parse_quantity(" 123K\x00"));
1111
var_dump(zend_test_zend_ini_parse_quantity(" 123\x00"));
1212

1313
--EXPECTF--
14-
Warning: Invalid quantity ' 123\x00K', interpreting as ' 123K' for backwards compatibility in %s on line %d
14+
Warning: Invalid quantity " 123\x00K", interpreting as " 123K" for backwards compatibility in %s on line %d
1515
int(125952)
1616

17-
Warning: Invalid quantity '\x00 123K': no valid leading digits, interpreting as '0' for backwards compatibility in %s on line %d
17+
Warning: Invalid quantity "\x00 123K": no valid leading digits, interpreting as "0" for backwards compatibility in %s on line %d
1818
int(0)
1919

20-
Warning: Invalid quantity ' \x00123K': no valid leading digits, interpreting as '0' for backwards compatibility in %s on line %d
20+
Warning: Invalid quantity " \x00123K": no valid leading digits, interpreting as "0" for backwards compatibility in %s on line %d
2121
int(0)
2222

23-
Warning: Invalid quantity ' 123\x00K', interpreting as ' 123K' for backwards compatibility in %s on line %d
23+
Warning: Invalid quantity " 123\x00K", interpreting as " 123K" for backwards compatibility in %s on line %d
2424
int(125952)
2525

26-
Warning: Invalid quantity ' 123K\x00': unknown multipler '\x00', interpreting as ' 123' for backwards compatibility in %s on line %d
26+
Warning: Invalid quantity " 123K\x00": unknown multipler "\x00", interpreting as " 123" for backwards compatibility in %s on line %d
2727
int(123)
2828

29-
Warning: Invalid quantity ' 123\x00': unknown multipler '\x00', interpreting as ' 123' for backwards compatibility in %s on line %d
29+
Warning: Invalid quantity " 123\x00": unknown multipler "\x00", interpreting as " 123" for backwards compatibility in %s on line %d
3030
int(123)

‎Zend/zend_ini.c

Lines changed: 120 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -541,94 +541,130 @@ ZEND_API bool zend_ini_parse_bool(zend_string *str)
541541
}
542542
}
543543

544-
ZEND_API zend_long zend_ini_parse_quantity(zend_string *value, zend_string **errstr) /* {{{ */
544+
static zend_long zend_ini_parse_quantity_internal(zend_string *value, bool signed_result, zend_string **errstr) /* {{{ */
545545
{
546546
char *digits_end = NULL;
547547
char *str = ZSTR_VAL(value);
548-
size_t str_len = ZSTR_LEN(value);
548+
char *str_end = &str[ZSTR_LEN(value)];
549+
char *digits = str;
550+
bool overflow = false;
551+
zend_ulong factor;
549552
smart_str invalid = {0};
550553
smart_str interpreted = {0};
551554
smart_str chr = {0};
552555

556+
/* Ignore leading whitespace. ZEND_STRTOL() also skips leading whitespaces,
557+
* but we need the position of the first non-whitespace later. */
558+
while (digits < str_end && zend_is_whitespace(*digits)) ++digits;
559+
553560
/* Ignore trailing whitespace */
554-
while (str_len && zend_is_whitespace(str[str_len-1])) --str_len;
561+
while (digits < str_end && zend_is_whitespace(*(str_end-1))) --str_end;
555562

556-
if (!str_len) {
563+
if (digits == str_end) {
557564
*errstr = NULL;
558565
return 0;
559566
}
560567

561-
/* Perform following multiplications on unsigned to avoid overflow UB.
562-
* For now overflow is silently ignored -- not clear what else can be
563-
* done here, especially as the final result of this function may be
564-
* used in an unsigned context (e.g. "memory_limit=3G", which overflows
565-
* zend_long on 32-bit, but not size_t). */
566-
zend_ulong retval = (zend_ulong) ZEND_STRTOL(str, &digits_end, 0);
568+
zend_ulong retval;
569+
errno = 0;
570+
571+
if (signed_result) {
572+
retval = (zend_ulong) ZEND_STRTOL(digits, &digits_end, 0);
573+
} else {
574+
retval = ZEND_STRTOUL(digits, &digits_end, 0);
575+
}
576+
577+
if (errno == ERANGE) {
578+
overflow = true;
579+
} else if (!signed_result) {
580+
/* ZEND_STRTOUL() does not report a range error when the subject starts
581+
* with a minus sign, so we check this here. Ignore "-1" as it is
582+
* commonly used as max value, for instance in memory_limit=-1. */
583+
if (digits[0] == '-' && !(digits_end - digits == 2 && digits_end == str_end && digits[1] == '1')) {
584+
overflow = true;
585+
}
586+
}
587+
588+
if (UNEXPECTED(digits_end == digits)) {
589+
/* No leading digits */
567590

568-
if (digits_end == str) {
569-
smart_str_append_escaped(&invalid, str, str_len);
591+
/* Escape the string to avoid null bytes and to make non-printable chars
592+
* visible */
593+
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
570594
smart_str_0(&invalid);
571595

572-
*errstr = zend_strpprintf(0, "Invalid quantity '%s': no valid leading digits, interpreting as '0' for backwards compatibility",
596+
*errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility",
573597
ZSTR_VAL(invalid.s));
574598

575599
smart_str_free(&invalid);
576600
return 0;
577601
}
578602

579603
/* Allow for whitespace between integer portion and any suffix character */
580-
while (digits_end < &str[str_len] && zend_is_whitespace(*digits_end)) ++digits_end;
604+
while (digits_end < str_end && zend_is_whitespace(*digits_end)) ++digits_end;
581605

582606
/* No exponent suffix. */
583-
if (digits_end == &str[str_len]) {
584-
*errstr = NULL;
585-
return retval;
586-
}
587-
588-
if (str_len>0) {
589-
switch (str[str_len-1]) {
590-
case 'g':
591-
case 'G':
592-
retval *= 1024;
593-
ZEND_FALLTHROUGH;
594-
case 'm':
595-
case 'M':
596-
retval *= 1024;
597-
ZEND_FALLTHROUGH;
598-
case 'k':
599-
case 'K':
600-
retval *= 1024;
601-
break;
602-
default:
603-
/* Unknown suffix */
604-
smart_str_append_escaped(&invalid, str, str_len);
605-
smart_str_0(&invalid);
606-
smart_str_append_escaped(&interpreted, str, digits_end - str);
607-
smart_str_0(&interpreted);
608-
smart_str_append_escaped(&chr, &str[str_len-1], 1);
609-
smart_str_0(&chr);
610-
611-
*errstr = zend_strpprintf(0, "Invalid quantity '%s': unknown multipler '%s', interpreting as '%s' for backwards compatibility",
612-
ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s));
613-
614-
smart_str_free(&invalid);
615-
smart_str_free(&interpreted);
616-
smart_str_free(&chr);
617-
618-
return retval;
607+
if (digits_end == str_end) {
608+
goto end;
609+
}
610+
611+
switch (*(str_end-1)) {
612+
case 'g':
613+
case 'G':
614+
factor = 1<<30;
615+
break;
616+
case 'm':
617+
case 'M':
618+
factor = 1<<20;
619+
break;
620+
case 'k':
621+
case 'K':
622+
factor = 1<<10;
623+
break;
624+
default:
625+
/* Unknown suffix */
626+
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
627+
smart_str_0(&invalid);
628+
smart_str_append_escaped(&interpreted, str, digits_end - str);
629+
smart_str_0(&interpreted);
630+
smart_str_append_escaped(&chr, str_end-1, 1);
631+
smart_str_0(&chr);
632+
633+
*errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multipler \"%s\", interpreting as \"%s\" for backwards compatibility",
634+
ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s));
635+
636+
smart_str_free(&invalid);
637+
smart_str_free(&interpreted);
638+
smart_str_free(&chr);
639+
640+
return retval;
641+
}
642+
643+
if (!overflow) {
644+
if (signed_result) {
645+
zend_long sretval = (zend_long)retval;
646+
if (sretval > 0) {
647+
overflow = (zend_long)retval > ZEND_LONG_MAX / (zend_long)factor;
648+
} else {
649+
overflow = (zend_long)retval < ZEND_LONG_MIN / (zend_long)factor;
650+
}
651+
} else {
652+
overflow = retval > ZEND_ULONG_MAX / factor;
619653
}
620654
}
621655

622-
if (digits_end < &str[str_len-1]) {
656+
retval *= factor;
657+
658+
if (UNEXPECTED(digits_end != str_end-1)) {
623659
/* More than one character in suffix */
624-
smart_str_append_escaped(&invalid, str, str_len);
660+
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
625661
smart_str_0(&invalid);
626662
smart_str_append_escaped(&interpreted, str, digits_end - str);
627663
smart_str_0(&interpreted);
628-
smart_str_append_escaped(&chr, &str[str_len-1], 1);
664+
smart_str_append_escaped(&chr, str_end-1, 1);
629665
smart_str_0(&chr);
630666

631-
*errstr = zend_strpprintf(0, "Invalid quantity '%s', interpreting as '%s%s' for backwards compatibility",
667+
*errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility",
632668
ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s));
633669

634670
smart_str_free(&invalid);
@@ -638,11 +674,41 @@ ZEND_API zend_long zend_ini_parse_quantity(zend_string *value, zend_string **err
638674
return (zend_long) retval;
639675
}
640676

677+
end:
678+
if (UNEXPECTED(overflow)) {
679+
smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value));
680+
smart_str_0(&invalid);
681+
682+
/* Not specifying the resulting value here because the caller may make
683+
* additional conversions. Not specifying the allowed range
684+
* because the caller may do narrower range checks. */
685+
*errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility",
686+
ZSTR_VAL(invalid.s));
687+
688+
smart_str_free(&invalid);
689+
smart_str_free(&interpreted);
690+
smart_str_free(&chr);
691+
692+
return (zend_long) retval;
693+
}
694+
641695
*errstr = NULL;
642696
return (zend_long) retval;
643697
}
644698
/* }}} */
645699

700+
ZEND_API zend_long zend_ini_parse_quantity(zend_string *value, zend_string **errstr) /* {{{ */
701+
{
702+
return zend_ini_parse_quantity_internal(value, true, errstr);
703+
}
704+
/* }}} */
705+
706+
zend_ulong zend_ini_parse_uquantity(zend_string *value, zend_string **errstr) /* {{{ */
707+
{
708+
return (zend_ulong) zend_ini_parse_quantity_internal(value, false, errstr);
709+
}
710+
/* }}} */
711+
646712
ZEND_INI_DISP(zend_ini_boolean_displayer_cb) /* {{{ */
647713
{
648714
int value;

‎Zend/zend_ini.h

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,27 +91,47 @@ ZEND_API zend_string *zend_ini_get_value(zend_string *name);
9191
ZEND_API bool zend_ini_parse_bool(zend_string *str);
9292

9393
/**
94-
* Parses a quantity
94+
* Parses an ini quantity
9595
*
96-
* value must be a string of digits optionally followed by a multiplier
97-
* character K, M, or G (for 2**10, 2**20, and 2**30, respectively).
96+
* The value parameter must be a string in the form
9897
*
99-
* The digits are parsed as decimal unless the first character is '0', in which
100-
* case they are parsed as octal.
98+
* sign? digits ws* multipler?
10199
*
102-
* Whitespaces before and after the multiplier are ignored.
100+
* with
101+
*
102+
* sign: [+-]
103+
* digit: [0-9]
104+
* digits: digit+
105+
* ws: [ \t\n\r\v\f]
106+
* multipler: [KMG]
107+
*
108+
* Leading and trailing whitespaces are ignored.
109+
*
110+
* If the string is empty or consists only of only whitespaces, 0 is returned.
111+
*
112+
* Digits is parsed as decimal unless the first digit is '0', in which case
113+
* digits is parsed as octal.
114+
*
115+
* The multiplier is case-insensitive. K, M, and G multiply the quantity by
116+
* 2**10, 2**20, and 2**30, respectively.
103117
*
104118
* For backwards compatibility, ill-formatted values are handled as follows:
105119
* - No leading digits: value is treated as '0'
106120
* - Invalid multiplier: multiplier is ignored
107121
* - Invalid characters between digits and multiplier: invalid characters are
108122
* ignored
123+
* - Integer overflow: The result of the overflow is returned
109124
*
110125
* In any of these cases an error string is stored in *errstr (caller must
111126
* release it), otherwise *errstr is set to NULL.
112127
*/
113128
ZEND_API zend_long zend_ini_parse_quantity(zend_string *value, zend_string **errstr);
114129

130+
/**
131+
* Unsigned variant of zend_ini_parse_quantity
132+
*/
133+
ZEND_API zend_ulong zend_ini_parse_uquantity(zend_string *value, zend_string **errstr);
134+
115135
ZEND_API zend_result zend_ini_register_displayer(const char *name, uint32_t name_length, void (*displayer)(zend_ini_entry *ini_entry, int type));
116136

117137
ZEND_API ZEND_INI_DISP(zend_ini_boolean_displayer_cb);

‎ext/zend_test/test.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,23 @@ static ZEND_FUNCTION(zend_test_zend_ini_parse_quantity)
373373
}
374374
}
375375

376+
static ZEND_FUNCTION(zend_test_zend_ini_parse_uquantity)
377+
{
378+
zend_string *str;
379+
zend_string *errstr;
380+
381+
ZEND_PARSE_PARAMETERS_START(1, 1)
382+
Z_PARAM_STR(str)
383+
ZEND_PARSE_PARAMETERS_END();
384+
385+
RETVAL_LONG((zend_long)zend_ini_parse_uquantity(str, &errstr));
386+
387+
if (errstr) {
388+
zend_error(E_WARNING, "%s", ZSTR_VAL(errstr));
389+
zend_string_release(errstr);
390+
}
391+
}
392+
376393
static ZEND_FUNCTION(namespaced_func)
377394
{
378395
ZEND_PARSE_PARAMETERS_NONE();

‎ext/zend_test/test.stub.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ function zend_get_current_func_name(): string {}
122122
function zend_call_method(object|string $obj_or_class, string $method, mixed $arg1 = UNKNOWN, mixed $arg2 = UNKNOWN): mixed {}
123123

124124
function zend_test_zend_ini_parse_quantity(string $str): int {}
125+
function zend_test_zend_ini_parse_uquantity(string $str): int {}
125126
}
126127

127128
namespace ZendTestNS {

0 commit comments

Comments
 (0)