From cfcc2a3fda3baaca83e9d5d605e578f445893f07 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 19 Jul 2024 22:52:03 +0200 Subject: [PATCH] Fix GH-15034: Integer overflow on stream_notification_callback byte_max parameter with files bigger than 2GB We were using atoi, which is only for integers. When the size does not fit in an integer this breaks. Use ZEND_STRTOUL instead. Also make sure invalid data isn't accidentally parsed into a file size. Closes GH-15035. --- NEWS | 2 ++ ext/standard/http_fopen_wrapper.c | 15 ++++++++-- ext/standard/tests/http/gh15034.phpt | 44 ++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/http/gh15034.phpt diff --git a/NEWS b/NEWS index ed4fb6b761c..f6599dc5fc4 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,8 @@ PHP NEWS - Streams: . Fixed bug GH-15028 (Memory leak in ext/phar/stream.c). (nielsdos) + . Fixed bug GH-15034 (Integer overflow on stream_notification_callback + byte_max parameter with files bigger than 2GB). (nielsdos) - Tidy: . Fix memory leaks in ext/tidy basedir restriction code. (nielsdos) diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index a9950fa6d48..dca176ad3f5 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -791,8 +791,19 @@ finish: } else if (!strncasecmp(http_header_line, "Content-Type:", sizeof("Content-Type:")-1)) { php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, http_header_value, 0); } else if (!strncasecmp(http_header_line, "Content-Length:", sizeof("Content-Length:")-1)) { - file_size = atoi(http_header_value); - php_stream_notify_file_size(context, file_size, http_header_line, 0); + /* https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length */ + const char *ptr = http_header_value; + /* must contain only digits, no + or - symbols */ + if (*ptr >= '0' && *ptr <= '9') { + char *endptr = NULL; + size_t parsed = ZEND_STRTOUL(ptr, &endptr, 10); + /* check whether there was no garbage in the header value and the conversion was successful */ + if (endptr && !*endptr) { + /* truncate for 32-bit such that no negative file sizes occur */ + file_size = MIN(parsed, ZEND_LONG_MAX); + php_stream_notify_file_size(context, file_size, http_header_line, 0); + } + } } else if ( !strncasecmp(http_header_line, "Transfer-Encoding:", sizeof("Transfer-Encoding:")-1) && !strncasecmp(http_header_value, "Chunked", sizeof("Chunked")-1) diff --git a/ext/standard/tests/http/gh15034.phpt b/ext/standard/tests/http/gh15034.phpt new file mode 100644 index 00000000000..c0841ef9544 --- /dev/null +++ b/ext/standard/tests/http/gh15034.phpt @@ -0,0 +1,44 @@ +--TEST-- +GH-15034 (Integer overflow on stream_notification_callback byte_max parameter with files bigger than 2GB) +--SKIPIF-- + +--INI-- +allow_url_fopen=1 +--FILE-- + $pid, 'uri' => $uri] = http_server($responses); + +$params = ['notification' => function( + int $notification_code, + int $severity, + ?string $message, + int $message_code, + int $bytes_transferred, + int $bytes_max +) { + global $max; + $max = $bytes_max; +}]; +$contextResource = stream_context_create([], $params); + +$resource = fopen($uri, 'r', false, $contextResource); +fclose($resource); + +http_server_kill($pid); + +var_dump($max); +?> +--EXPECT-- +int(3000000000)