Edit report at http://bugs.php.net/bug.php?id=51023&edit=1
ID: 51023
Updated by: geiss...@php.net
Reported by: geissert at debian dot org
Summary: ext/filter/tests/046.phpt fails, does not detect int
overflow (with -O2 gcc 4.4)
-Status: Open
+Status: Closed
Type: Bug
Package: Filter related
Operating System: *
PHP Version: 5.3SVN-2010-02-12
Assigned To: geissert
New Comment:
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.
Previous Comments:
------------------------------------------------------------------------
[2010-03-06 19:54:58] geiss...@php.net
Automatic comment from SVN on behalf of geissert
Revision: http://svn.php.net/viewvc/?view=revision&revision=295896
Log: Detect overflows before they occur in the filter extension (bug
#51023)
Thanks to Sean Finney for the patch
------------------------------------------------------------------------
[2010-02-25 21:53:40] 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 <sean...@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-02-23 13:04:48] j...@php.net
See also bug #51008
------------------------------------------------------------------------
[2010-02-20 20:56:44] geiss...@php.net
Further investigation revealed that the bug occurs with gcc 4.4 and
optimisation -02. Without optimisation the code still works.
------------------------------------------------------------------------
[2010-02-11 23:31:02] 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)
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/bug.php?id=51023&edit=1
