From b132da7f9df39c1774997f21016c522b676a6ab0 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 13 May 2020 09:36:52 +0200 Subject: [PATCH] Fix #77423: parse_url() will deliver a wrong host to user To avoid that `parse_url()` returns an erroneous host, which would be valid for `FILTER_VALIDATE_URL`, we make sure that only userinfo which is valid according to RFC 3986 is treated as such. For consistency with the existing url parsing code, we use ctype functions, although that is not necessarily correct. --- ext/standard/tests/strings/url_t.phpt | 6 ++-- ext/standard/tests/url/bug77423.phpt | 30 +++++++++++++++++++ .../tests/url/parse_url_basic_001.phpt | 6 ++-- .../tests/url/parse_url_basic_003.phpt | 2 +- .../tests/url/parse_url_basic_005.phpt | 2 +- .../tests/url/parse_url_unterminated.phpt | 6 ++-- ext/standard/url.c | 24 +++++++++++++-- 7 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 ext/standard/tests/url/bug77423.phpt diff --git a/ext/standard/tests/strings/url_t.phpt b/ext/standard/tests/strings/url_t.phpt index caa93cb9cf..dc13018b14 100644 --- a/ext/standard/tests/strings/url_t.phpt +++ b/ext/standard/tests/strings/url_t.phpt @@ -589,15 +589,13 @@ $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(7) { +--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) { ["scheme"]=> string(4) "http" ["host"]=> - string(11) "www.php.net" + string(26) "secret@hideout@www.php.net" ["port"]=> int(80) - ["user"]=> - string(14) "secret@hideout" ["path"]=> string(10) "/index.php" ["query"]=> diff --git a/ext/standard/tests/url/bug77423.phpt b/ext/standard/tests/url/bug77423.phpt new file mode 100644 index 0000000000..be03fe95e2 --- /dev/null +++ b/ext/standard/tests/url/bug77423.phpt @@ -0,0 +1,30 @@ +--TEST-- +Bug #77423 (parse_url() will deliver a wrong host to user) +--FILE-- + +--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/url/parse_url_basic_001.phpt b/ext/standard/tests/url/parse_url_basic_001.phpt index f3abd703b2..89b1b7b3d6 100644 --- a/ext/standard/tests/url/parse_url_basic_001.phpt +++ b/ext/standard/tests/url/parse_url_basic_001.phpt @@ -514,15 +514,13 @@ 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(7) { +--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) { ["scheme"]=> string(4) "http" ["host"]=> - string(11) "www.php.net" + string(26) "secret@hideout@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 1eb64d6a1b..da35b92782 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(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: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 dfbe7e7971..731a4adb31 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 : string(14) "secret@hideout" +--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : NULL --> 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 7c9150a513..f859fc7d40 100644 --- a/ext/standard/tests/url/parse_url_unterminated.phpt +++ b/ext/standard/tests/url/parse_url_unterminated.phpt @@ -522,15 +522,13 @@ 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(7) { +--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) { ["scheme"]=> string(4) "http" ["host"]=> - string(11) "www.php.net" + string(26) "secret@hideout@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 a33091a86b..113e010024 100644 --- a/ext/standard/url.c +++ b/ext/standard/url.c @@ -92,6 +92,22 @@ static const char *binary_strcspn(const char *s, const char *e, const char *char return e; } +static int is_userinfo_valid(const char *str, size_t len) +{ + const char *valid = "-._~!$&'()*+,;=:"; + const char *p = str; + while (p - str < len) { + if (isalpha(*p) || isdigit(*p) || strchr(valid, *p)) { + p++; + } else if (*p == '%' && p - str <= len - 3 && isdigit(*(p+1)) && isxdigit(*(p+2))) { + p += 3; + } else { + return 0; + } + } + return 1; +} + /* {{{ php_url_parse */ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length) { @@ -233,13 +249,17 @@ parse_host: ret->pass = zend_string_init(pp, (p-pp), 0); php_replace_controlchars_ex(ZSTR_VAL(ret->pass), ZSTR_LEN(ret->pass)); } else { - ret->user = zend_string_init(s, (p-s), 0); - php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user)); + if (!is_userinfo_valid(s, p-s)) { + goto check_port; + } + 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, -- 2.50.1