Signed integer overflow in ext/standard/scanf.c

[YuanchengJiang]

Description

The following code:

<?php
try {
    sprintf('%2147483648$s, %2$s %1$s', "a", "b");
} catch (ValueError $e) {
    echo $e->getMessage(), "\n";
}
sscanf($e,$e,$e);
?>

Resulted in this output:

/php-src/ext/standard/scanf.c:364:21: runtime error: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /php-src/ext/standard/scanf.c:364:21

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

[cmb69]

Simpler reproducer (which triggers a segfault for me; PHP 8.2+):

sscanf('hello','%2147483648$s');

So this is apparently an issue with the argnum specifier, which is way to large. Quick fix (without much thinking and further checking):

 ext/standard/scanf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ext/standard/scanf.c b/ext/standard/scanf.c
index 78ecc1642c..cf232e84b3 100644
--- a/ext/standard/scanf.c
+++ b/ext/standard/scanf.c
@@ -352,6 +352,9 @@ PHPAPI int ValidateFormat(char *format, int numVars, int *totalSubs)
 			 * in the same format string.
 			 */
 			value = ZEND_STRTOUL(format-1, &end, 10);
+			if (value < 0) {
+				goto badIndex;
+			}
 			if (*end != '$') {
 				goto notXpg;
 			}

[devnexen]

That should work, or you can always check if the zend_ulong val fits the 0/INT_MAX boundaries before assigning to value otherwise but ...

[cmb69]

Maybe we should assign objIndex only after having done the bounds checking, i.e. (maybe there are off-by-one errors; should be checked more thoroughly):

 ext/standard/scanf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/standard/scanf.c b/ext/standard/scanf.c
index 78ecc1642c..408e5ede88 100644
--- a/ext/standard/scanf.c
+++ b/ext/standard/scanf.c
@@ -361,8 +361,7 @@ PHPAPI int ValidateFormat(char *format, int numVars, int *totalSubs)
 			if (gotSequential) {
 				goto mixedXPG;
 			}
-			objIndex = value - 1;
-			if ((objIndex < 0) || (numVars && (objIndex >= numVars))) {
+			if ((value < 1) || (numVars && (value > numVars))) {
 				goto badIndex;
 			} else if (numVars == 0) {
 				/*
@@ -382,6 +381,7 @@ PHPAPI int ValidateFormat(char *format, int numVars, int *totalSubs)
 
 				xpgSize = (xpgSize > value) ? xpgSize : value;
 			}
+			objIndex = value - 1;
 			goto xpgCheckDone;
 		}