ID: 51023
Comment by: seanius at debian dot org
Reported By: geissert at debian dot org
Status: Open
Bug Type: Filter related
Operating System: *
PHP Version: 5.3SVN-2010-02-12
New Comment:
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
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
--FILE--
---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)
Previous Comments:
[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://bug
