Fix improper signed overflow detection in filter extension
authorSean Finney <seanius@debian.org>
Thu, 25 Feb 2010 21:02:52 +0000 (22:02 +0100)
committerSean Finney <seanius@debian.org>
Thu, 25 Feb 2010 21:41:03 +0000 (22:41 +0100)
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 [new file with mode: 0644]
debian/patches/series

diff --git a/debian/patches/filter_validate_int.patch b/debian/patches/filter_validate_int.patch
new file mode 100644 (file)
index 0000000..8c33995
--- /dev/null
@@ -0,0 +1,124 @@
+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)
index 097fa4a..e7c1a45 100644 (file)
@@ -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