Skip to content

Commit 2d19c92

Browse files
committed
Make php_url_parse_ex() respect length argument
This should fix all out-of-bounds reads that could previously occur if the string passed to php_url_parse_ex() is not NUL terminated.
1 parent f0f68c7 commit 2d19c92

File tree

1 file changed

+28
-20
lines changed

1 file changed

+28
-20
lines changed

ext/standard/url.c

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ PHPAPI php_url *php_url_parse_ex(char const *str, int length)
104104
ue = s + length;
105105

106106
/* parse scheme */
107-
if ((e = memchr(s, ':', length)) && (e - s)) {
107+
if ((e = memchr(s, ':', length)) && e != s) {
108108
/* validate scheme */
109109
p = s;
110110
while (p < e) {
@@ -119,7 +119,7 @@ PHPAPI php_url *php_url_parse_ex(char const *str, int length)
119119
p++;
120120
}
121121

122-
if (*(e + 1) == '\0') { /* only scheme is available */
122+
if (e + 1 == ue) { /* only scheme is available */
123123
ret->scheme = estrndup(s, (e - s));
124124
php_replace_controlchars_ex(ret->scheme, (e - s));
125125
return ret;
@@ -134,11 +134,11 @@ PHPAPI php_url *php_url_parse_ex(char const *str, int length)
134134
* correctly parse things like a.com:80
135135
*/
136136
p = e + 1;
137-
while (isdigit(*p)) {
137+
while (p < ue && isdigit(*p)) {
138138
p++;
139139
}
140140

141-
if ((*p == '\0' || *p == '/') && (p - e) < 7) {
141+
if ((p == ue || *p == '/') && (p - e) < 7) {
142142
goto parse_port;
143143
}
144144

@@ -151,14 +151,14 @@ PHPAPI php_url *php_url_parse_ex(char const *str, int length)
151151
ret->scheme = estrndup(s, (e-s));
152152
php_replace_controlchars_ex(ret->scheme, (e - s));
153153

154-
if (*(e+2) == '/') {
154+
if (e + 2 < ue && *(e + 2) == '/') {
155155
s = e + 3;
156156
if (!strncasecmp("file", ret->scheme, sizeof("file"))) {
157-
if (*(e + 3) == '/') {
157+
if (e + 3 < ue && *(e + 3) == '/') {
158158
/* support windows drive letters as in:
159159
file:///c:/somedir/file.txt
160160
*/
161-
if (*(e + 5) == ':') {
161+
if (e + 5 < ue && *(e + 5) == ':') {
162162
s = e + 4;
163163
}
164164
goto just_path;
@@ -174,41 +174,51 @@ PHPAPI php_url *php_url_parse_ex(char const *str, int length)
174174
p = e + 1;
175175
pp = p;
176176

177-
while (pp-p < 6 && isdigit(*pp)) {
177+
while (pp < ue && pp - p < 6 && isdigit(*pp)) {
178178
pp++;
179179
}
180180

181-
if (pp - p > 0 && pp - p < 6 && (*pp == '/' || *pp == '\0')) {
181+
if (pp - p > 0 && pp - p < 6 && (pp == ue || *pp == '/')) {
182182
long port;
183183
memcpy(port_buf, p, (pp - p));
184184
port_buf[pp - p] = '\0';
185185
port = strtol(port_buf, NULL, 10);
186186
if (port > 0 && port <= 65535) {
187187
ret->port = (unsigned short) port;
188-
if (*s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
188+
if (s + 1 < ue && *s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
189189
s += 2;
190190
}
191191
} else {
192192
STR_FREE(ret->scheme);
193193
efree(ret);
194194
return NULL;
195195
}
196-
} else if (p == pp && *pp == '\0') {
196+
} else if (p == pp && pp == ue) {
197197
STR_FREE(ret->scheme);
198198
efree(ret);
199199
return NULL;
200-
} else if (*s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
200+
} else if (s + 1 < ue && *s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
201201
s += 2;
202202
} else {
203203
goto just_path;
204204
}
205-
} else if (*s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
205+
} else if (s + 1 < ue && *s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
206206
s += 2;
207207
} else {
208208
goto just_path;
209209
}
210210

211-
e = s + strcspn(s, "/?#");
211+
/* Binary-safe strcspn(s, "/?#") */
212+
e = ue;
213+
if ((p = memchr(s, '/', e - s))) {
214+
e = p;
215+
}
216+
if ((p = memchr(s, '?', e - s))) {
217+
e = p;
218+
}
219+
if ((p = memchr(s, '#', e - s))) {
220+
e = p;
221+
}
212222

213223
/* check for login and password */
214224
if ((p = zend_memrchr(s, '@', (e-s)))) {
@@ -228,18 +238,16 @@ PHPAPI php_url *php_url_parse_ex(char const *str, int length)
228238
}
229239

230240
/* check for port */
231-
if (*s == '[' && *(e-1) == ']') {
241+
if (s < ue && *s == '[' && *(e-1) == ']') {
232242
/* Short circuit portscan,
233243
we're dealing with an
234244
IPv6 embedded address */
235-
p = s;
245+
p = NULL;
236246
} else {
237-
/* memrchr is a GNU specific extension
238-
Emulate for wide compatibility */
239-
for(p = e; p >= s && *p != ':'; p--);
247+
p = zend_memrchr(s, ':', (e-s));
240248
}
241249

242-
if (p >= s && *p == ':') {
250+
if (p) {
243251
if (!ret->port) {
244252
p++;
245253
if (e-p > 5) { /* port cannot be longer then 5 characters */

0 commit comments

Comments
 (0)