Skip to content

Commit 54dbd3e

Browse files
committed
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.
1 parent 2e9e706 commit 54dbd3e

12 files changed

+33
-13
lines changed

ext/standard/tests/url/parse_url_basic_001.phpt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,11 @@ echo "Done";
854854
string(19) "filter={"id":"123"}"
855855
}
856856

857+
--> %:x: array(1) {
858+
["path"]=>
859+
string(3) "%:x"
860+
}
861+
857862
--> http:///blah.com: bool(false)
858863

859864
--> http://:80: bool(false)

ext/standard/tests/url/parse_url_basic_002.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ echo "Done";
112112
--> : NULL
113113
--> / : NULL
114114
--> /rest/Users?filter={"id":"123"} : NULL
115+
--> %:x : NULL
115116
--> http:///blah.com : bool(false)
116117
--> http://:80 : bool(false)
117118
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_003.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : NULL
112112
--> / : NULL
113113
--> /rest/Users?filter={"id":"123"} : NULL
114+
--> %:x : NULL
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_004.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : NULL
112112
--> / : NULL
113113
--> /rest/Users?filter={"id":"123"} : NULL
114+
--> %:x : NULL
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_005.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : NULL
112112
--> / : NULL
113113
--> /rest/Users?filter={"id":"123"} : NULL
114+
--> %:x : NULL
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_006.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : NULL
112112
--> / : NULL
113113
--> /rest/Users?filter={"id":"123"} : NULL
114+
--> %:x : NULL
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_007.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : string(0) ""
112112
--> / : string(1) "/"
113113
--> /rest/Users?filter={"id":"123"} : string(11) "/rest/Users"
114+
--> %:x : string(3) "%:x"
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_008.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : NULL
112112
--> / : NULL
113113
--> /rest/Users?filter={"id":"123"} : string(19) "filter={"id":"123"}"
114+
--> %:x : NULL
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_basic_009.phpt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ echo "Done";
111111
--> : NULL
112112
--> / : NULL
113113
--> /rest/Users?filter={"id":"123"} : NULL
114+
--> %:x : NULL
114115
--> http:///blah.com : bool(false)
115116
--> http://:80 : bool(false)
116117
--> http://user@:80 : bool(false)

ext/standard/tests/url/parse_url_unterminated.phpt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,11 @@ echo "Done";
856856
string(19) "filter={"id":"123"}"
857857
}
858858

859+
--> %:x: array(1) {
860+
["path"]=>
861+
string(3) "%:x"
862+
}
863+
859864
--> http:///blah.com: bool(false)
860865

861866
--> http://:80: bool(false)

ext/standard/tests/url/urls.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ $urls = array(
9191
'',
9292
'/',
9393
'/rest/Users?filter={"id":"123"}',
94+
'%:x',
9495

9596
// Severely malformed URLs that do not parse:
9697
'http:///blah.com',

ext/standard/url.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,17 @@ PHPAPI php_url *php_url_parse(char const *str)
9191
return php_url_parse_ex(str, strlen(str));
9292
}
9393

94+
static const char *binary_strcspn(const char *s, const char *e, const char *chars) {
95+
while (*chars) {
96+
const char *p = memchr(s, *chars, e - s);
97+
if (p) {
98+
e = p;
99+
}
100+
chars++;
101+
}
102+
return e;
103+
}
104+
94105
/* {{{ php_url_parse
95106
*/
96107
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)
109120
while (p < e) {
110121
/* scheme = 1*[ lowalpha | digit | "+" | "-" | "." ] */
111122
if (!isalpha(*p) && !isdigit(*p) && *p != '+' && *p != '.' && *p != '-') {
112-
if (e + 1 < ue && e < s + strcspn(s, "?#")) {
123+
if (e + 1 < ue && e < binary_strcspn(s, ue, "?#")) {
113124
goto parse_port;
114125
} else if (s + 1 < ue && *s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
115126
s += 2;
@@ -209,18 +220,8 @@ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
209220
goto just_path;
210221
}
211222

212-
parse_host:
213-
/* Binary-safe strcspn(s, "/?#") */
214-
e = ue;
215-
if ((p = memchr(s, '/', e - s))) {
216-
e = p;
217-
}
218-
if ((p = memchr(s, '?', e - s))) {
219-
e = p;
220-
}
221-
if ((p = memchr(s, '#', e - s))) {
222-
e = p;
223-
}
223+
parse_host:
224+
e = binary_strcspn(s, ue, "/?#");
224225

225226
/* check for login and password */
226227
if ((p = zend_memrchr(s, '@', (e-s)))) {

0 commit comments

Comments
 (0)