]> granicus.if.org Git - php/commitdiff
Fixed bug #61218 (the previous patch was not enough restritive on fcgi name string...
authorJerome Loyet <fat@php.net>
Sat, 26 May 2012 17:37:09 +0000 (19:37 +0200)
committerJerome Loyet <fat@php.net>
Sat, 26 May 2012 17:37:09 +0000 (19:37 +0200)
sapi/fpm/fpm/fastcgi.c

index 9df26f11cdb497108850d4b5ad42089d8129312b..e2e208aa7fd3b1b8980e54cd44affee06400aa39 100644 (file)
@@ -395,12 +395,39 @@ static inline size_t fcgi_get_params_len( int *result, unsigned char *p, unsigne
        return ret;
 }
 
+static inline int fcgi_param_get_eff_len( unsigned char *p, unsigned char *end, uint *eff_len)
+{
+       int ret = 1;
+       int zero_found = 0;
+       *eff_len = 0;
+       for (; p != end; ++p) {
+               if (*p == '\0') {
+                       zero_found = 1;
+               }
+               else {
+                       if (zero_found) {
+                               ret = 0;
+                               break;
+                       }
+                       if (*eff_len < ((uint)-1)) {
+                               ++*eff_len;
+                       }
+                       else {
+                               ret = 0;
+                               break;
+                       }
+               }
+       }
+       return ret;
+}
+
 static int fcgi_get_params(fcgi_request *req, unsigned char *p, unsigned char *end)
 {
        char buf[128];
        char *tmp = buf;
        size_t buf_size = sizeof(buf);
        int name_len, val_len;
+       uint eff_name_len;
        char *s;
        int ret = 1;
        size_t bytes_consumed;
@@ -427,26 +454,35 @@ static int fcgi_get_params(fcgi_request *req, unsigned char *p, unsigned char *e
                        break;
                }
 
-               if (name_len >= buf_size-1) {
-                       if (name_len > ((uint)-1)-64) { 
+               /*
+                * get the effective length of the name in case it's not a valid string
+                * don't do this on the value because it can be binary data
+                */
+               if (!fcgi_param_get_eff_len(p, p+name_len, &eff_name_len)){
+                       /* Malicious request */
+                       ret = 0;
+                       break;
+               }
+               if (eff_name_len >= buf_size-1) {
+                       if (eff_name_len > ((uint)-1)-64) { 
                                ret = 0;
                                break;
                        }
-                       buf_size = name_len + 64;
+                       buf_size = eff_name_len + 64;
                        tmp = (tmp == buf ? emalloc(buf_size): erealloc(tmp, buf_size));
                        if (tmp == NULL) {
                                ret = 0;
                                break;
                        }
                }
-               memcpy(tmp, p, name_len);
-               tmp[name_len] = 0;
+               memcpy(tmp, p, eff_name_len);
+               tmp[eff_name_len] = 0;
                s = estrndup((char*)p + name_len, val_len);
                if (s == NULL) {
                        ret = 0;
                        break;
                }
-               zend_hash_update(req->env, tmp, name_len+1, &s, sizeof(char*), NULL);
+               zend_hash_update(req->env, tmp, eff_name_len+1, &s, sizeof(char*), NULL);
                p += name_len + val_len;
        }
        if (tmp != buf && tmp != NULL) {