[Pkg-php-commits] [php/debian-sid] Fix improper signed overflow detection in filter extension

[Sean Finney]

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.

Closes: #570287
---
 debian/patches/filter_validate_int.patch |  124 ++++++++++++++++++++++++++++++
 debian/patches/series                    |    1 +
 2 files changed, 125 insertions(+), 0 deletions(-)
 create mode 100644 debian/patches/filter_validate_int.patch

diff --git a/debian/patches/filter_validate_int.patch b/debian/patches/filter_validate_int.patch
new file mode 100644
index 0000000..8c33995
--- /dev/null
+++ b/debian/patches/filter_validate_int.patch
@@ -0,0 +1,124 @@
+From: Sean Finney <seanius at 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)
diff --git a/debian/patches/series b/debian/patches/series
index 097fa4a..e7c1a45 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -44,3 +44,4 @@ dont-gitclean-in-build.patch
 broken_5.3_test-posix_uname.patch
 shtool_mkdir_-p_-race-condition.patch
 qdbm-is-usr_include_qdbm.patch
+filter_validate_int.patch
-- 
1.6.3.3