212 lines
8.3 KiB
Diff
212 lines
8.3 KiB
Diff
From 4a89e726bd4d0571991dc22a9a1ad4509e8fe347 Mon Sep 17 00:00:00 2001
|
|
From: "Christoph M. Becker" <cmbecker69@gmx.de>
|
|
Date: Tue, 19 Jan 2021 11:23:25 +0100
|
|
Subject: [PATCH] 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>
|
|
---
|
|
ext/filter/logical_filters.c | 23 +++++++++++++++++++
|
|
.../tests/url => filter/tests}/bug77423.phpt | 15 ------------
|
|
ext/standard/tests/strings/url_t.phpt | 6 +++--
|
|
.../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 | 6 +----
|
|
8 files changed, 38 insertions(+), 28 deletions(-)
|
|
rename ext/{standard/tests/url => filter/tests}/bug77423.phpt (53%)
|
|
|
|
diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c
|
|
index 076d2479461f..1cf345dbb58a 100644
|
|
--- a/ext/filter/logical_filters.c
|
|
+++ b/ext/filter/logical_filters.c
|
|
@@ -532,6 +532,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;
|
|
@@ -592,6 +608,13 @@ void php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */
|
|
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
|
|
similarity index 53%
|
|
rename from ext/standard/tests/url/bug77423.phpt
|
|
rename to ext/filter/tests/bug77423.phpt
|
|
index be03fe95e24e..761c7c359a92 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 f564f59f0632..79ff3bc4a8e3 100644
|
|
--- a/ext/standard/tests/strings/url_t.phpt
|
|
+++ b/ext/standard/tests/strings/url_t.phpt
|
|
@@ -575,13 +575,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 57014f08391a..7ffd001856c8 100644
|
|
--- a/ext/standard/tests/url/parse_url_basic_001.phpt
|
|
+++ b/ext/standard/tests/url/parse_url_basic_001.phpt
|
|
@@ -506,13 +506,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 e7cd24c9698c..8b0e7eb87500 100644
|
|
--- a/ext/standard/tests/url/parse_url_basic_003.phpt
|
|
+++ b/ext/standard/tests/url/parse_url_basic_003.phpt
|
|
@@ -68,7 +68,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 8617d37059ec..a5ca381a691f 100644
|
|
--- a/ext/standard/tests/url/parse_url_basic_005.phpt
|
|
+++ b/ext/standard/tests/url/parse_url_basic_005.phpt
|
|
@@ -68,7 +68,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 1377cb7735f0..8f46eb3dbedf 100644
|
|
--- a/ext/standard/tests/url/parse_url_unterminated.phpt
|
|
+++ b/ext/standard/tests/url/parse_url_unterminated.phpt
|
|
@@ -508,13 +508,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 157d61466e69..9743a00166b1 100644
|
|
--- a/ext/standard/url.c
|
|
+++ b/ext/standard/url.c
|
|
@@ -260,9 +260,6 @@ 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);
|
|
php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
|
|
}
|
|
@@ -271,7 +268,6 @@ parse_host:
|
|
s = p + 1;
|
|
}
|
|
|
|
-check_port:
|
|
/* check for port */
|
|
if (s < ue && *s == '[' && *(e-1) == ']') {
|
|
/* Short circuit portscan,
|