Re: Security Patch for #33605

I've just applied a bit different patch. Thanks for the heads up. On 25.04.2006 14:25, Mike Bretz wrote:

According to recent security alerts I have made a patch against 5.1 CVS for bug 33605, which is already CLOSED but not fixed. See attachment for patch and test file. Somebody can review it and merge it into 5.1 / HEAD? mike ------------------------------------------------------------------------ --TEST-- Bug #33605 (substr_compare crashes) --FILE-- <?php $res = substr_compare("aa", "a", -99999999, 0, 0); var_dump($res); ?> --EXPECTF-- Warning: substr_compare(): The length must be greater than zero. in %sbug33605.php on line %d bool(false) ------------------------------------------------------------------------ diff -uw php-src.orig/ext/standard/string.c php-src/ext/standard/string.c --- php-src.orig/ext/standard/string.c 2006-04-03 11:14:33.000000000 +0200

+++ php-src/ext/standard/string.c       2006-04-25 12:01:42.000000000 +0200

@@ -4884,13 +4884,19 @@

                RETURN_FALSE;
        }
 -       if ((offset + len) >= s1_len) {
-               php_error_docref(NULL TSRMLS_CC, E_WARNING, "The start position cannot exceed initial string length.");
+       if (len <= 0) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "The length must be greater than zero.");
                RETURN_FALSE;
        }
         if (offset < 0) {
                offset = s1_len + offset;
+               if (offset < 0) offset = 0;
+       }

+

+       if ((offset + len) >= s1_len) {
+               php_error_docref(NULL TSRMLS_CC, E_WARNING, "The start position cannot exceed initial string length.");
+               RETURN_FALSE;
        }
         cmp_len = (uint) (len ? len : MAX(s2_len, (s1_len - offset)));

-- Wbr, Antony Dovgal