Alternative fix for bug 77423

That bug report originally was about `parse_url()` misbehaving, but the
security aspect was actually only regarding `FILTER_VALIDATE_URL`.
Since the changes to `parse_url_ex()` apparently affect userland code
which is relying on the sloppy URL parsing[1], this alternative
restores the old parsing behavior, but ensures that the userinfo is
checked for correctness for `FILTER_VALIDATE_URL`.

[1] <https://github.com/php/php-src/commit/5174de7cd33c3d4fa591c9c93859ff9989b07e8c#commitcomment-45967652>

diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c
index 076d2479461fdb9c726b9a1446d8b42576748cdd..1cf345dbb58af441d9cd21fd86498a9158e3f51f 100644 (file)
--- a/ext/filter/logical_filters.c
+++ b/ext/filter/logical_filters.c
@@ -532,6 +532,22 @@ void php_filter_validate_domain(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
 }
 /* }}} */
 
+static int is_userinfo_valid(zend_string *str)
+{
+       const char *valid = "-._~!$&'()*+,;=:";
+       const char *p = ZSTR_VAL(str);
+       while (p - ZSTR_VAL(str) < ZSTR_LEN(str)) {
+               if (isalpha(*p) || isdigit(*p) || strchr(valid, *p)) {
+                       p++;
+               } else if (*p == '%' && p - ZSTR_VAL(str) <= ZSTR_LEN(str) - 3 && isdigit(*(p+1)) && isxdigit(*(p+2))) {
+                       p += 3;
+               } else {
+                       return 0;
+               }
+       }
+       return 1;
+}
+
 void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
 {
        php_url *url;
@@ -592,6 +608,13 @@ bad_url:
                php_url_free(url);
                RETURN_VALIDATION_FAILED
        }
+
+       if (url->user != NULL && !is_userinfo_valid(url->user)) {
+               php_url_free(url);
+               RETURN_VALIDATION_FAILED
+
+       }
+
        php_url_free(url);
 }
 /* }}} */

diff --git a/ext/standard/tests/url/bug77423.phpt b/ext/filter/tests/bug77423.phpt
similarity index 53%
rename from ext/standard/tests/url/bug77423.phpt
rename to ext/filter/tests/bug77423.phpt
index be03fe95e24e2015c19d60138a24c8d35289fe5d..761c7c359a9299a267cc5dff1ce16afae15d560f 100644 (file)
--- a/ext/standard/tests/url/bug77423.phpt
+++ b/ext/filter/tests/bug77423.phpt
@@ -8,23 +8,8 @@ $urls = array(
 );
 foreach ($urls as $url) {
     var_dump(filter_var($url, FILTER_VALIDATE_URL));
-    var_dump(parse_url($url));
 }
 ?>
 --EXPECT--
 bool(false)
-array(3) {
-  ["scheme"]=>
-  string(4) "http"
-  ["host"]=>
-  string(19) "php.net\@aliyun.com"
-  ["path"]=>
-  string(7) "/aaa.do"
-}
 bool(false)
-array(2) {
-  ["scheme"]=>
-  string(5) "https"
-  ["host"]=>
-  string(26) "example.com\uFF03@bing.com"
-}

diff --git a/ext/standard/tests/strings/url_t.phpt b/ext/standard/tests/strings/url_t.phpt
index f564f59f0632226b386265eaa250b83fef730a53..79ff3bc4a8e3df2ba8307627aeed3143d5ff21c3 100644 (file)
--- a/ext/standard/tests/strings/url_t.phpt
+++ b/ext/standard/tests/strings/url_t.phpt
@@ -575,13 +575,15 @@ $sample_urls = array (
   string(16) "some_page_ref123"
 }
 
---> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
+--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
   ["scheme"]=>
   string(4) "http"
   ["host"]=>
-  string(26) "secret@hideout@www.php.net"
+  string(11) "www.php.net"
   ["port"]=>
   int(80)
+  ["user"]=>
+  string(14) "secret@hideout"
   ["path"]=>
   string(10) "/index.php"
   ["query"]=>

diff --git a/ext/standard/tests/url/parse_url_basic_001.phpt b/ext/standard/tests/url/parse_url_basic_001.phpt
index 57014f08391aebe911c46f5d2ebd1ec66e6b347c..7ffd001856c858b58e0e9c2233a3fcac72646379 100644 (file)
--- a/ext/standard/tests/url/parse_url_basic_001.phpt
+++ b/ext/standard/tests/url/parse_url_basic_001.phpt
@@ -506,13 +506,15 @@ echo "Done";
   string(16) "some_page_ref123"
 }
 
---> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
+--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
   ["scheme"]=>
   string(4) "http"
   ["host"]=>
-  string(26) "secret@hideout@www.php.net"
+  string(11) "www.php.net"
   ["port"]=>
   int(80)
+  ["user"]=>
+  string(14) "secret@hideout"
   ["path"]=>
   string(10) "/index.php"
   ["query"]=>

diff --git a/ext/standard/tests/url/parse_url_basic_003.phpt b/ext/standard/tests/url/parse_url_basic_003.phpt
index e7cd24c9698c809b479dd5eb6a516dca6f555cb0..8b0e7eb87500ab20d8e3fb76afe363bbbd456ca0 100644 (file)
--- a/ext/standard/tests/url/parse_url_basic_003.phpt
+++ b/ext/standard/tests/url/parse_url_basic_003.phpt
@@ -68,7 +68,7 @@ echo "Done";
 --> http://secret:@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(11) "www.php.net"
 --> http://:hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(11) "www.php.net"
 --> http://secret:hideout@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(11) "www.php.net"
---> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(26) "secret@hideout@www.php.net"
+--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(11) "www.php.net"
 --> http://secret:hid:out@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(11) "www.php.net"
 --> nntp://news.php.net   : string(12) "news.php.net"
 --> ftp://ftp.gnu.org/gnu/glic/glibc.tar.gz   : string(11) "ftp.gnu.org"

diff --git a/ext/standard/tests/url/parse_url_basic_005.phpt b/ext/standard/tests/url/parse_url_basic_005.phpt
index 8617d37059ec909048986f97eb85c841ab73e869..a5ca381a691fe7b0e099b6c558b871f4a025e3e4 100644 (file)
--- a/ext/standard/tests/url/parse_url_basic_005.phpt
+++ b/ext/standard/tests/url/parse_url_basic_005.phpt
@@ -68,7 +68,7 @@ echo "Done";
 --> http://secret:@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(6) "secret"
 --> http://:hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(0) ""
 --> http://secret:hideout@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(6) "secret"
---> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : NULL
+--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(14) "secret@hideout"
 --> http://secret:hid:out@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123   : string(6) "secret"
 --> nntp://news.php.net   : NULL
 --> ftp://ftp.gnu.org/gnu/glic/glibc.tar.gz   : NULL

diff --git a/ext/standard/tests/url/parse_url_unterminated.phpt b/ext/standard/tests/url/parse_url_unterminated.phpt
index 1377cb7735f0e9e3b813edc5bb9dcba2280feeb1..8f46eb3dbedffc55e2d26d9464ee26e195ce8a12 100644 (file)
--- a/ext/standard/tests/url/parse_url_unterminated.phpt
+++ b/ext/standard/tests/url/parse_url_unterminated.phpt
@@ -508,13 +508,15 @@ echo "Done";
   string(16) "some_page_ref123"
 }
 
---> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
+--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
   ["scheme"]=>
   string(4) "http"
   ["host"]=>
-  string(26) "secret@hideout@www.php.net"
+  string(11) "www.php.net"
   ["port"]=>
   int(80)
+  ["user"]=>
+  string(14) "secret@hideout"
   ["path"]=>
   string(10) "/index.php"
   ["query"]=>

diff --git a/ext/standard/url.c b/ext/standard/url.c
index 157d61466e698bf3a9e679ac3385a88d56eff1fa..9743a00166b127db4d47fce27ba31f9e036571ac 100644 (file)
--- a/ext/standard/url.c
+++ b/ext/standard/url.c
@@ -260,17 +260,13 @@ parse_host:
                        ret->pass = zend_string_init(pp, (p-pp), 0);
                        php_replace_controlchars_ex(ZSTR_VAL(ret->pass), ZSTR_LEN(ret->pass));
                } else {
-            if (!is_userinfo_valid(s, p-s)) {
-                goto check_port;
-            }
-            ret->user = zend_string_init(s, (p-s), 0);
+                       ret->user = zend_string_init(s, (p-s), 0);
                        php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
                }
 
                s = p + 1;
        }
 
-check_port:
        /* check for port */
        if (s < ue && *s == '[' && *(e-1) == ']') {
                /* Short circuit portscan,