Skip to content

Commit 773e85a

Browse files
author
Jerome Loyet
committed
Fixed bug #61218 (the previous patch was not enough restritive on fcgi name string checks)
1 parent 60cca8b commit 773e85a

File tree

1 file changed

+42
-6
lines changed

1 file changed

+42
-6
lines changed

sapi/fpm/fpm/fastcgi.c

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -395,12 +395,39 @@ static inline size_t fcgi_get_params_len( int *result, unsigned char *p, unsigne
395395
return ret;
396396
}
397397

398+
static inline int fcgi_param_get_eff_len( unsigned char *p, unsigned char *end, uint *eff_len)
399+
{
400+
int ret = 1;
401+
int zero_found = 0;
402+
*eff_len = 0;
403+
for (; p != end; ++p) {
404+
if (*p == '\0') {
405+
zero_found = 1;
406+
}
407+
else {
408+
if (zero_found) {
409+
ret = 0;
410+
break;
411+
}
412+
if (*eff_len < ((uint)-1)) {
413+
++*eff_len;
414+
}
415+
else {
416+
ret = 0;
417+
break;
418+
}
419+
}
420+
}
421+
return ret;
422+
}
423+
398424
static int fcgi_get_params(fcgi_request *req, unsigned char *p, unsigned char *end)
399425
{
400426
char buf[128];
401427
char *tmp = buf;
402428
size_t buf_size = sizeof(buf);
403429
int name_len, val_len;
430+
uint eff_name_len;
404431
char *s;
405432
int ret = 1;
406433
size_t bytes_consumed;
@@ -427,26 +454,35 @@ static int fcgi_get_params(fcgi_request *req, unsigned char *p, unsigned char *e
427454
break;
428455
}
429456

430-
if (name_len >= buf_size-1) {
431-
if (name_len > ((uint)-1)-64) {
457+
/*
458+
* get the effective length of the name in case it's not a valid string
459+
* don't do this on the value because it can be binary data
460+
*/
461+
if (!fcgi_param_get_eff_len(p, p+name_len, &eff_name_len)){
462+
/* Malicious request */
463+
ret = 0;
464+
break;
465+
}
466+
if (eff_name_len >= buf_size-1) {
467+
if (eff_name_len > ((uint)-1)-64) {
432468
ret = 0;
433469
break;
434470
}
435-
buf_size = name_len + 64;
471+
buf_size = eff_name_len + 64;
436472
tmp = (tmp == buf ? emalloc(buf_size): erealloc(tmp, buf_size));
437473
if (tmp == NULL) {
438474
ret = 0;
439475
break;
440476
}
441477
}
442-
memcpy(tmp, p, name_len);
443-
tmp[name_len] = 0;
478+
memcpy(tmp, p, eff_name_len);
479+
tmp[eff_name_len] = 0;
444480
s = estrndup((char*)p + name_len, val_len);
445481
if (s == NULL) {
446482
ret = 0;
447483
break;
448484
}
449-
zend_hash_update(req->env, tmp, name_len+1, &s, sizeof(char*), NULL);
485+
zend_hash_update(req->env, tmp, eff_name_len+1, &s, sizeof(char*), NULL);
450486
p += name_len + val_len;
451487
}
452488
if (tmp != buf && tmp != NULL) {

0 commit comments

Comments
 (0)