php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #51023 ext/filter/tests/046.phpt fails, does not detect int overflow (with -O2 gcc 4.4)
Submitted: 2010-02-11 23:31 UTC Modified: 2010-03-06 19:56 UTC
From: geissert at debian dot org Assigned: geissert (profile)
Status: Closed Package: Filter related
PHP Version: 5.3SVN-2010-02-12 OS: *
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: geissert at debian dot org
New email:
PHP Version: OS:

 

 [2010-02-11 23:31 UTC] geissert at debian dot org
Description:
------------
The filter fails to detect an integer overflow and passes the FILTER_VALIDATE_INT test. The problem is caused because php_filter_parse_int uses a long to detect the overflow, which of course doesn't have the same size of an integer.

This can be fixed by making ctx_value an integer in both php_filter_parse_int and php_filter_int (and for correctness, not setting Z_TYPE_P(value) to IS_LONG).


Reproduce code:
---------------
// the current test:
$s = sprintf("%d", PHP_INT_MAX);
var_dump(is_long(filter_var($s, FILTER_VALIDATE_INT)));

$s = sprintf("%.0f", PHP_INT_MAX+1);
var_dump(filter_var($s, FILTER_VALIDATE_INT));

$s = sprintf("%d", -PHP_INT_MAX);
var_dump(is_long(filter_var($s, FILTER_VALIDATE_INT)));

Expected result:
----------------
bool(true)
bool(false)
bool(true)


Actual result:
--------------
bool(true)
int(-2147483648)
bool(true)

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2010-02-20 20:56 UTC] geissert@php.net
Further investigation revealed that the bug occurs with gcc 4.4 and optimisation -02. Without optimisation the code still works.

 [2010-02-23 13:04 UTC] jani@php.net
See also bug #51008
 [2010-02-25 21:53 UTC] seanius at debian dot org
Here's the patch i've cobbled together.  in case it doesn't cut/paste okay, it's also available at: http://git.debian.org/?p=pkg-php/php.git;a=commitdiff;h=3061d111de130df7388cc78e26b63cc105574775

From: Sean Finney <seanius@debian.org>
Subject: Fix improper signed overflow detection in filter extension

The existing filter code relied on detecting invalid long integers by
examining computed values for wraparound.  This is not defined behavior
in any C standard, and in fact recent versions of gcc will optimize out
such checks resulting in invalid code.

This patch therefore changes how the overflow/underflow conditions are
detected, using more reliable arithmetic.  It also fixes another bug, that
the minimum integer value (-PHP_INT_MAX)-1 could not be detected properly.

This patch also includes an update to the test case that detects such
overflows, adding much more thorough and descriptive checking.

Bug: http://bugs.php.net/bug.php?id=51023
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=570287
--- php.orig/ext/filter/logical_filters.c
+++ php/ext/filter/logical_filters.c
@@ -68,7 +68,7 @@
 
 static int php_filter_parse_int(const char *str, unsigned int str_len, long *ret TSRMLS_DC) { /* {{{ */
 	long ctx_value;
-	int sign = 0;
+	int sign = 0, digit = 0;
 	const char *end = str + str_len;
 
 	switch (*str) {
@@ -82,7 +82,7 @@ static int php_filter_parse_int(const ch
 
 	/* must start with 1..9*/
 	if (str < end && *str >= '1' && *str <= '9') {
-		ctx_value = ((*(str++)) - '0');
+		ctx_value = ((sign)?-1:1) * ((*(str++)) - '0');
 	} else {
 		return -1;
 	}
@@ -95,19 +95,18 @@ static int php_filter_parse_int(const ch
 
 	while (str < end) {
 		if (*str >= '0' && *str <= '9') {
-			ctx_value = (ctx_value * 10) + (*(str++) - '0');
+			digit = (*(str++) - '0');
+			if ( (!sign) && ctx_value <= (LONG_MAX-digit)/10 ) {
+				ctx_value = (ctx_value * 10) + digit;
+			} else if ( sign && ctx_value >= (LONG_MIN+digit)/10) {
+				ctx_value = (ctx_value * 10) - digit;
+			} else {
+				return -1;
+			}
 		} else {
 			return -1;
 		}
 	}
-	if (sign) {
-		ctx_value = -ctx_value;
-		if (ctx_value > 0) { /* overflow */
-			return -1;
-		}
-	} else if (ctx_value < 0) { /* overflow */
-		return -1;
-	}
 
 	*ret = ctx_value;
 	return 1;
--- php.orig/ext/filter/tests/046.phpt
+++ php/ext/filter/tests/046.phpt
@@ -4,16 +4,46 @@ Integer overflow
 <?php if (!extension_loaded("filter")) die("skip"); ?>
 --FILE--
 <?php
-$s = sprintf("%d", PHP_INT_MAX);
-var_dump(is_long(filter_var($s, FILTER_VALIDATE_INT)));
+$max = sprintf("%d", PHP_INT_MAX);
+switch($max) {
+case "2147483647": /* 32-bit systems */
+	$min = "-2147483648";
+	$overflow = "2147483648";
+	$underflow = "-2147483649";
+	break;
+case "9223372036854775807": /* 64-bit systems */
+	$min = "-9223372036854775808";
+	$overflow = "9223372036854775808";
+	$underflow = "-9223372036854775809";
+	break;
+default:
+	die("failed: unknown value for PHP_MAX_INT");
+	break;
+}
 
-$s = sprintf("%.0f", PHP_INT_MAX+1);
-var_dump(filter_var($s, FILTER_VALIDATE_INT));
+function test_validation($val, $msg) {
+	$f = filter_var($val, FILTER_VALIDATE_INT);
+	echo "$msg filtered: "; var_dump($f); // filtered value (or false)
+	echo "$msg is_long: "; var_dump(is_long($f)); // test validation
+	echo "$msg equal: "; var_dump($val == $f); // test equality of result
+}
 
-$s = sprintf("%d", -PHP_INT_MAX);
-var_dump(is_long(filter_var($s, FILTER_VALIDATE_INT)));
+// PHP_INT_MAX
+test_validation($max, "max");
+test_validation($overflow, "overflow");
+test_validation($min, "min");
+test_validation($underflow, "underflow");
 ?>
---EXPECT--
-bool(true)
-bool(false)
-bool(true)
+--EXPECTF--
+max filtered: int(%d)
+max is_long: bool(true)
+max equal: bool(true)
+overflow filtered: bool(false)
+overflow is_long: bool(false)
+overflow equal: bool(false)
+min filtered: int(-%d)
+min is_long: bool(true)
+min equal: bool(true)
+underflow filtered: bool(false)
+underflow is_long: bool(false)
+underflow equal: bool(false)
 [2010-03-06 19:54 UTC] geissert@php.net
Automatic comment from SVN on behalf of geissert
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=295896
Log: Detect overflows before they occur in the filter extension (bug #51023)
Thanks to Sean Finney for the patch
 [2010-03-06 19:56 UTC] geissert@php.net
-Status: Open +Status: Closed
 [2010-03-06 19:56 UTC] geissert@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.


 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Dec 10 13:01:27 2024 UTC