summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristoph M. Becker <cmbecker69@gmx.de>2021-01-19 11:23:25 +0100
committerGabriel Caruso <carusogabriel34@gmail.com>2021-02-02 07:45:26 -0300
commit605d9f55ad0da0608364f7a7629f5df921dfe2ec (patch)
tree405dba9b05ebb76c44c0bbb307636d52d39a66f5
parent5e21867c552a861f3b59593fdb12268f983507de (diff)
downloadphp-git-605d9f55ad0da0608364f7a7629f5df921dfe2ec.tar.gz
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>
-rw-r--r--ext/filter/logical_filters.c23
-rw-r--r--ext/filter/tests/bug77423.phpt (renamed from ext/standard/tests/url/bug77423.phpt)15
-rw-r--r--ext/standard/tests/strings/url_t.phpt6
-rw-r--r--ext/standard/tests/url/parse_url_basic_001.phpt6
-rw-r--r--ext/standard/tests/url/parse_url_basic_003.phpt2
-rw-r--r--ext/standard/tests/url/parse_url_basic_005.phpt2
-rw-r--r--ext/standard/tests/url/parse_url_unterminated.phpt6
-rw-r--r--ext/standard/url.c6
8 files changed, 38 insertions, 28 deletions
diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c
index 392156b539..4a66d685e9 100644
--- a/ext/filter/logical_filters.c
+++ b/ext/filter/logical_filters.c
@@ -556,6 +556,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;
@@ -611,6 +627,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
index be03fe95e2..761c7c359a 100644
--- 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 dc13018b14..caa93cb9cf 100644
--- a/ext/standard/tests/strings/url_t.phpt
+++ b/ext/standard/tests/strings/url_t.phpt
@@ -589,13 +589,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 89b1b7b3d6..f3abd703b2 100644
--- a/ext/standard/tests/url/parse_url_basic_001.phpt
+++ b/ext/standard/tests/url/parse_url_basic_001.phpt
@@ -514,13 +514,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 da35b92782..1eb64d6a1b 100644
--- a/ext/standard/tests/url/parse_url_basic_003.phpt
+++ b/ext/standard/tests/url/parse_url_basic_003.phpt
@@ -62,7 +62,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 731a4adb31..dfbe7e7971 100644
--- a/ext/standard/tests/url/parse_url_basic_005.phpt
+++ b/ext/standard/tests/url/parse_url_basic_005.phpt
@@ -62,7 +62,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 f859fc7d40..7c9150a513 100644
--- a/ext/standard/tests/url/parse_url_unterminated.phpt
+++ b/ext/standard/tests/url/parse_url_unterminated.phpt
@@ -522,13 +522,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 8866744506..f60294f9da 100644
--- a/ext/standard/url.c
+++ b/ext/standard/url.c
@@ -249,17 +249,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,