Fix binary-safety of parse_url

php_parse_url() is intended to support strings that are not zero terminated. We can't use strcspn in the implementation. As we have two uses of strcspn, add a helper.
php · Sep 2, 2020 · 54dbd3e · 54dbd3e
1 parent 2e9e706
commit 54dbd3e
Show file tree

Hide file tree

Showing 12 changed files with 33 additions and 13 deletions.
diff --git a/ext/standard/tests/url/parse_url_basic_001.phpt b/ext/standard/tests/url/parse_url_basic_001.phpt
@@ -854,6 +854,11 @@ echo "Done";
   string(19) "filter={"id":"123"}"
 }
 
+--> %:x: array(1) {
+  ["path"]=>
+  string(3) "%:x"
+}
+
 --> http:///blah.com: bool(false)
 
 --> http://:80: bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_002.phpt b/ext/standard/tests/url/parse_url_basic_002.phpt
@@ -112,6 +112,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : NULL
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_003.phpt b/ext/standard/tests/url/parse_url_basic_003.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : NULL
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_004.phpt b/ext/standard/tests/url/parse_url_basic_004.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : NULL
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_005.phpt b/ext/standard/tests/url/parse_url_basic_005.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : NULL
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_006.phpt b/ext/standard/tests/url/parse_url_basic_006.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : NULL
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_007.phpt b/ext/standard/tests/url/parse_url_basic_007.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : string(0) ""
 --> /   : string(1) "/"
 --> /rest/Users?filter={"id":"123"}   : string(11) "/rest/Users"
+--> %:x   : string(3) "%:x"
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_008.phpt b/ext/standard/tests/url/parse_url_basic_008.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : string(19) "filter={"id":"123"}"
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_basic_009.phpt b/ext/standard/tests/url/parse_url_basic_009.phpt
@@ -111,6 +111,7 @@ echo "Done";
 -->    : NULL
 --> /   : NULL
 --> /rest/Users?filter={"id":"123"}   : NULL
+--> %:x   : NULL
 --> http:///blah.com   : bool(false)
 --> http://:80   : bool(false)
 --> http://user@:80   : bool(false)

diff --git a/ext/standard/tests/url/parse_url_unterminated.phpt b/ext/standard/tests/url/parse_url_unterminated.phpt
@@ -856,6 +856,11 @@ echo "Done";
   string(19) "filter={"id":"123"}"
 }
 
+--> %:x: array(1) {
+  ["path"]=>
+  string(3) "%:x"
+}
+
 --> http:///blah.com: bool(false)
 
 --> http://:80: bool(false)

diff --git a/ext/standard/tests/url/urls.inc b/ext/standard/tests/url/urls.inc
@@ -91,6 +91,7 @@ $urls = array(
 '',
 '/',
 '/rest/Users?filter={"id":"123"}',
+'%:x',
 
 // Severely malformed URLs that do not parse:
 'http:///blah.com',

diff --git a/ext/standard/url.c b/ext/standard/url.c
@@ -91,6 +91,17 @@ PHPAPI php_url *php_url_parse(char const *str)
 	return php_url_parse_ex(str, strlen(str));
 }
 
+static const char *binary_strcspn(const char *s, const char *e, const char *chars) {
+	while (*chars) {
+		const char *p = memchr(s, *chars, e - s);
+		if (p) {
+			e = p;
+		}
+		chars++;
+	}
+	return e;
+}
+
 /* {{{ php_url_parse
  */
 PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
@@ -109,7 +120,7 @@ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
 		while (p < e) {
 			/* scheme = 1*[ lowalpha | digit | "+" | "-" | "." ] */
 			if (!isalpha(*p) && !isdigit(*p) && *p != '+' && *p != '.' && *p != '-') {
-				if (e + 1 < ue && e < s + strcspn(s, "?#")) {
+				if (e + 1 < ue && e < binary_strcspn(s, ue, "?#")) {
 					goto parse_port;
 				} else if (s + 1 < ue && *s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
 					s += 2;
@@ -209,18 +220,8 @@ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
 		goto just_path;
 	}
 
-	parse_host:
-	/* Binary-safe strcspn(s, "/?#") */
-	e = ue;
-	if ((p = memchr(s, '/', e - s))) {
-		e = p;
-	}
-	if ((p = memchr(s, '?', e - s))) {
-		e = p;
-	}
-	if ((p = memchr(s, '#', e - s))) {
-		e = p;
-	}
+parse_host:
+	e = binary_strcspn(s, ue, "/?#");
 
 	/* check for login and password */
 	if ((p = zend_memrchr(s, '@', (e-s)))) {