Bug #78454 [Ana->Csd]: Multiple consecutive numeric separators in bin/hex numbers cause fatal error

Edit report at https://bugs.php.net/bug.php?id=78454&edit=1 ID: 78454 Updated by: cmb@php.net Reported by: mattacosta at gmail dot com Summary: Multiple consecutive numeric separators in bin/hex numbers cause fatal error -Status: Analyzed +Status: Closed Type: Bug Package: Scripting Engine problem PHP Version: 7.4.0beta4 Block user comment: N Private report: N New Comment: Automatic comment on behalf of theodorejb@outlook.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=1a78bdab276a9e34aa1ae00a184538e2d0dacdcd Log: Fix #78454: Consecutive numeric separators cause OOM error Previous Comments: ------------------------------------------------------------------------ [2019-08-25 14:59:28] theodorejb at outlook dot com The following pull request has been associated: Patch Name: Fix bug #78454 On GitHub: https://github.com/php/php-src/pull/4618 Patch: https://github.com/php/php-src/pull/4618.patch ------------------------------------------------------------------------ [2019-08-25 08:58:13] nikic@php.net > Here's my initial attempt at a fix. I'm not sure if it's the right approach or > not, but it does produce a nicer error message! Checking the length is fine, but it should not throw an error. In this case 0x0 is still a well-formed number and _ is a well-formed label -- the actual error occurs during parsing. Alternatively the number tokens could be redefined to support arbitrary _ and make cases like __ or trailing _ an explicit error. That will give nicer error messages, but this needs a corresponding change in the regexes to avoid inconsistent behavior. ------------------------------------------------------------------------ [2019-08-25 05:50:26] theodorejb at outlook dot com It seems that multiple consecutive numeric separators aren't the only problem. If I use any invalid character after 0x0_ or 0b0_ I get the following fatal error: Fatal error: Possible integer overflow in memory allocation (1 * 18446744073709551615 + 1). Here's my initial attempt at a fix. I'm not sure if it's the right approach or not, but it does produce a nicer error message! Zend/zend_language_scanner.l | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/Zend/zend_language_scanner.l b/Zend/zend_language_scanner.l index 2e21ee7952..901ae1085a 100644 --- a/Zend/zend_language_scanner.l +++ b/Zend/zend_language_scanner.l @@ -1776,8 +1776,16 @@ NEWLINE ("\r"|"\n"|"\r\n") /* Skip any leading 0s */ while (*bin == '0' || *bin == '_') { + if (len < 1) { + zend_throw_exception(zend_ce_parse_error, "Invalid numeric literal", 0); + if (PARSER_MODE()) { + RETURN_TOKEN(T_ERROR); + } + RETURN_TOKEN_WITH_VAL(T_LNUMBER); + } else { + --len; + } ++bin; - --len; } contains_underscores = (memchr(bin, '_', len) != NULL); @@ -1893,8 +1901,16 @@ NEWLINE ("\r"|"\n"|"\r\n") /* Skip any leading 0s */ while (*hex == '0' || *hex == '_') { + if (len < 1) { + zend_throw_exception(zend_ce_parse_error, "Invalid numeric literal", 0); + if (PARSER_MODE()) { + RETURN_TOKEN(T_ERROR); + } + RETURN_TOKEN_WITH_VAL(T_LNUMBER); + } else { + --len; + } ++hex; - --len; } contains_underscores = (memchr(hex, '_', len) != NULL); ------------------------------------------------------------------------ [2019-08-25 05:01:31] theodorejb at outlook dot com Is something wrong with the re2c regex? It seems strange that the BNUM/HNUM code even runs when there are invalid characters in the literal. Do we have to duplicate syntax checks in the BNUM/HNUM code? ------------------------------------------------------------------------ [2019-08-24 22:55:49] requinix@php.net > Fatal error: Possible integer overflow in memory allocation (0 + 32) 1. Multiple consecutive underscores are not supposed to be allowed 2. Parser is matching HNUM as only the "0x0" 3. HNUM code skips *all* leading zeros and underscores, goes beyond "0x0" length assumed, len underflows ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=78454 -- Edit this bug report at https://bugs.php.net/bug.php?id=78454&edit=1