Skip to content

Commit 5174de7

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 9bf43c4 commit 5174de7

File tree

7 files changed

+60
-16
lines changed

7 files changed

+60
-16
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@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
592+
--> http://secret@hideout@www.php.net: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\uFF03@bing.com",
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\uFF03@bing.com"
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@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
517+
--> http://secret@hideout@www.php.net: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://:hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
6464
--> http://secret:hideout@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
65-
--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(11) "www.php.net"
65+
--> 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"
6666
--> http://secret:hid:out@www.php.net: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://:hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(0) ""
6464
--> http://secret:hideout@www.php.net/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(6) "secret"
65-
--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : string(14) "secret@hideout"
65+
--> http://secret@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123 : NULL
6666
--> http://secret:hid:out@www.php.net: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@hideout@www.php.net:80/index.php?test=1&test2=char&test3=mixesCI#some_page_ref123: array(7) {
525+
--> http://secret@hideout@www.php.net: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: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,22 @@ 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 */
96112
PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
97113
{
@@ -233,13 +249,17 @@ 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 = zend_string_init(s, (p-s), 0);
256+
php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
238257
}
239258

240259
s = p + 1;
241260
}
242261

262+
check_port:
243263
/* check for port */
244264
if (s < ue && *s == '[' && *(e-1) == ']') {
245265
/* Short circuit portscan,

0 commit comments

Comments
 (0)