Skip to content

Commit 0f97a92

Browse files
cmb69smalyshev
cmb69
authored and
smalyshev
committed
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.
1 parent dfb9e03 commit 0f97a92

File tree

7 files changed

+62
-17
lines changed

7 files changed

+62
-17
lines changed

‎ext/standard/tests/strings/url_t.phpt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -589,15 +589,13 @@ $sample_urls = array (
589589
string(16) "some_page_ref123"
590590
}
591591

592-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
592+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
593593
["scheme"]=>
594594
string(4) "http"
595595
["host"]=>
596-
string(11) "www.php.net"
596+
string(26) "secret@hideout@www.php.net"
597597
["port"]=>
598598
int(80)
599-
["user"]=>
600-
string(14) "secret@hideout"
601599
["path"]=>
602600
string(10) "/index.php"
603601
["query"]=>

‎ext/standard/tests/url/bug77423.phpt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
Bug #77423 (parse_url() will deliver a wrong host to user)
3+
--FILE--
4+
<?php
5+
$urls = array(
6+
"http://php.net\@aliyun.com/aaa.do",
7+
"https://example.com\u[email protected]",
8+
);
9+
foreach ($urls as $url) {
10+
var_dump(filter_var($url, FILTER_VALIDATE_URL));
11+
var_dump(parse_url($url));
12+
}
13+
?>
14+
--EXPECT--
15+
bool(false)
16+
array(3) {
17+
["scheme"]=>
18+
string(4) "http"
19+
["host"]=>
20+
string(19) "php.net\@aliyun.com"
21+
["path"]=>
22+
string(7) "/aaa.do"
23+
}
24+
bool(false)
25+
array(2) {
26+
["scheme"]=>
27+
string(5) "https"
28+
["host"]=>
29+
string(26) "example.com\[email protected]"
30+
}

‎ext/standard/tests/url/parse_url_basic_001.phpt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -514,15 +514,13 @@ echo "Done";
514514
string(16) "some_page_ref123"
515515
}
516516

517-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
517+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
518518
["scheme"]=>
519519
string(4) "http"
520520
["host"]=>
521-
string(11) "www.php.net"
521+
string(26) "secret@hideout@www.php.net"
522522
["port"]=>
523523
int(80)
524-
["user"]=>
525-
string(14) "secret@hideout"
526524
["path"]=>
527525
string(10) "/index.php"
528526
["query"]=>

‎ext/standard/tests/url/parse_url_basic_003.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ echo "Done";
6262
--> http://secret:@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
6363
--> http://:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
6464
--> http://secret:[email protected]/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
65-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
65+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(26) "secret@hideout@www.php.net"
6666
--> http://secret:hid:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
6767
--> nntp://news.php.net : string(12) "news.php.net"
6868
--> ftp://ftp.gnu.org/gnu/glic/glibc.tar.gz : string(11) "ftp.gnu.org"

‎ext/standard/tests/url/parse_url_basic_005.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ echo "Done";
6262
--> http://secret:@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
6363
--> http://:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(0) ""
6464
--> http://secret:[email protected]/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
65-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(14) "secret@hideout"
65+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : NULL
6666
--> http://secret:hid:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
6767
--> nntp://news.php.net : NULL
6868
--> ftp://ftp.gnu.org/gnu/glic/glibc.tar.gz : NULL

‎ext/standard/tests/url/parse_url_unterminated.phpt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -522,15 +522,13 @@ echo "Done";
522522
string(16) "some_page_ref123"
523523
}
524524

525-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
525+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
526526
["scheme"]=>
527527
string(4) "http"
528528
["host"]=>
529-
string(11) "www.php.net"
529+
string(26) "secret@hideout@www.php.net"
530530
["port"]=>
531531
int(80)
532-
["user"]=>
533-
string(14) "secret@hideout"
534532
["path"]=>
535533
string(10) "/index.php"
536534
["query"]=>

‎ext/standard/url.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,24 @@ static const char *binary_strcspn(const char *s, const char *e, const char *char
9292
return e;
9393
}
9494

95+
static int is_userinfo_valid(const char *str, size_t len)
96+
{
97+
const char *valid = "-._~!$&'()*+,;=:";
98+
const char *p = str;
99+
while (p - str < len) {
100+
if (isalpha(*p) || isdigit(*p) || strchr(valid, *p)) {
101+
p++;
102+
} else if (*p == '%' && p - str <= len - 3 && isdigit(*(p+1)) && isxdigit(*(p+2))) {
103+
p += 3;
104+
} else {
105+
return 0;
106+
}
107+
}
108+
return 1;
109+
}
110+
95111
/* {{{ php_url_parse */
96-
PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
112+
PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
97113
{
98114
zend_bool has_port;
99115
return php_url_parse_ex2(str, length, &has_port);
@@ -233,13 +249,18 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, zend_bool *has
233249
ret->pass = zend_string_init(pp, (p-pp), 0);
234250
php_replace_controlchars_ex(ZSTR_VAL(ret->pass), ZSTR_LEN(ret->pass));
235251
} else {
236-
ret->user = zend_string_init(s, (p-s), 0);
237-
php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
252+
if (!is_userinfo_valid(s, p-s)) {
253+
goto check_port;
254+
}
255+
ret->user = estrndup(s, (p-s));
256+
php_replace_controlchars_ex(ret->user, (p-s));
257+
238258
}
239259

240260
s = p + 1;
241261
}
242262

263+
check_port:
243264
/* check for port */
244265
if (s < ue && *s == '[' && *(e-1) == ']') {
245266
/* Short circuit portscan,

0 commit comments

Comments
 (0)