Skip to content

Commit 2d3d724

Browse files
cmb69smalyshev
authored andcommitted
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 662083f commit 2d3d724

File tree

7 files changed

+59
-14
lines changed

7 files changed

+59
-14
lines changed

ext/standard/tests/strings/url_t.phpt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -575,15 +575,13 @@ $sample_urls = array (
575575
string(16) "some_page_ref123"
576576
}
577577

578-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
578+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
579579
["scheme"]=>
580580
string(4) "http"
581581
["host"]=>
582-
string(11) "www.php.net"
582+
string(26) "secret@hideout@www.php.net"
583583
["port"]=>
584584
int(80)
585-
["user"]=>
586-
string(14) "secret@hideout"
587585
["path"]=>
588586
string(10) "/index.php"
589587
["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
@@ -506,15 +506,13 @@ echo "Done";
506506
string(16) "some_page_ref123"
507507
}
508508

509-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
509+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
510510
["scheme"]=>
511511
string(4) "http"
512512
["host"]=>
513-
string(11) "www.php.net"
513+
string(26) "secret@hideout@www.php.net"
514514
["port"]=>
515515
int(80)
516-
["user"]=>
517-
string(14) "secret@hideout"
518516
["path"]=>
519517
string(10) "/index.php"
520518
["query"]=>

ext/standard/tests/url/parse_url_basic_003.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ echo "Done";
6868
--> http://secret:@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
6969
--> http://:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
7070
--> http://secret:[email protected]/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
71-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
71+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(26) "secret@hideout@www.php.net"
7272
--> http://secret:hid:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
7373
--> nntp://news.php.net : string(12) "news.php.net"
7474
--> 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
@@ -68,7 +68,7 @@ echo "Done";
6868
--> http://secret:@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
6969
--> http://:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(0) ""
7070
--> http://secret:[email protected]/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
71-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(14) "secret@hideout"
71+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : NULL
7272
--> http://secret:hid:[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
7373
--> nntp://news.php.net : NULL
7474
--> 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
@@ -508,15 +508,13 @@ echo "Done";
508508
string(16) "some_page_ref123"
509509
}
510510

511-
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
511+
--> http://secret@[email protected]:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(6) {
512512
["scheme"]=>
513513
string(4) "http"
514514
["host"]=>
515-
string(11) "www.php.net"
515+
string(26) "secret@hideout@www.php.net"
516516
["port"]=>
517517
int(80)
518-
["user"]=>
519-
string(14) "secret@hideout"
520518
["path"]=>
521519
string(10) "/index.php"
522520
["query"]=>

ext/standard/url.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,22 @@ PHPAPI php_url *php_url_parse(char const *str)
9292
return php_url_parse_ex(str, strlen(str));
9393
}
9494

95+
static int is_userinfo_valid(const char *str, size_t len)
96+
{
97+
char *valid = "-._~!$&'()*+,;=:";
98+
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
96112
*/
97113
PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
@@ -235,13 +251,18 @@ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
235251
ret->pass = estrndup(pp, (p-pp));
236252
php_replace_controlchars_ex(ret->pass, (p-pp));
237253
} else {
254+
if (!is_userinfo_valid(s, p-s)) {
255+
goto check_port;
256+
}
238257
ret->user = estrndup(s, (p-s));
239258
php_replace_controlchars_ex(ret->user, (p-s));
259+
240260
}
241261

242262
s = p + 1;
243263
}
244264

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

0 commit comments

Comments
 (0)