Fixed: Fix CRLF injection in redirect and header (swoole#3541)

* Fix CRLF injection in `redirect` and `header` The previous fix (swoole#3539) was incomplete. The added check is not enforced when `ucwords` is set to false. * Fix typo # Conflicts: # swoole_http_response.cc
Yurunsoft · Aug 31, 2020 · bf20cc7 · bf20cc7
1 parent 4688ff3
commit bf20cc7
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 15 deletions.
diff --git a/swoole_http_response.cc b/swoole_http_response.cc
@@ -861,6 +861,19 @@ bool swoole_http_response_set_header(http_context *ctx, const char *k, size_t kl
         php_swoole_error(E_WARNING, "header key is too long");
         return false;
     }
+    /* new line/NUL character safety check */
+    uint32_t i;
+    for (i = 0; i < vlen; i++) {
+        /* RFC 7230 ch. 3.2.4 deprecates folding support */
+        if (v[i] == '\n' || v[i] == '\r') {
+            php_swoole_error(E_WARNING, "Header may not contain more than a single header, new line detected");
+            return false;
+        }
+        if (v[i] == '\0') {
+            php_swoole_error(E_WARNING, "Header may not contain NUL bytes");
+            return false;
+        }
+    }
     zval *zheader = swoole_http_init_and_read_property(swoole_http_response_ce, ctx->response.zobject, &ctx->response.zheader, ZEND_STRL("header"));
     if (ucwords)
     {
@@ -882,20 +895,6 @@ bool swoole_http_response_set_header(http_context *ctx, const char *k, size_t kl
         }
         else
         {
-            /* new line/NUL character safety check */
-            uint32_t i;
-            for (i = 0; i < vlen; i++) {
-                /* RFC 7230 ch. 3.2.4 deprecates folding support */
-                if (v[i] == '\n' || v[i] == '\r') {
-                    php_swoole_error(E_WARNING, "Header may not contain more than a single header, new line detected");
-                    return false;
-                }
-                if (v[i] == '\0') {
-                    php_swoole_error(E_WARNING, "Header may not contain NUL bytes");
-                    return false;
-                }
-            }
-
             add_assoc_stringl_ex(zheader, key_buf, klen, (char *) v, vlen);
         }
     }

diff --git a/tests/swoole_http_server_coro/check_http_header_crlf.phpt b/tests/swoole_http_server_coro/check_http_header_crlf.phpt
@@ -18,7 +18,7 @@ $pm->parentFunc = function () use ($pm) {
         $client = new Client('127.0.0.1', $pm->getFreePort());
         $client->get('/?r=AAA%0d%0amalicious-header:injected');
         $headers = $client->getHeaders();
-        Assert::false(isset($headers['amalicious-header']));
+        Assert::false(isset($headers['malicious-header']));
         $client->close();
         $pm->kill();
         echo "DONE\n";