CVE-2024-24752: Uncontrolled Resource Consumption (CWE-400)
value: Bref enable serverless PHP on AWS Lambda. When Bref is used with the Event-Driven Function runtime and the handler is a `RequestHandlerInterface`, then the Lambda event is converted to a PSR7 object. During the conversion process, if the request is a MultiPart, each part is parsed and for each which contains a file, it is extracted and saved in `/tmp` with a random filename starting with `bref_upload_`. The flow mimics what plain PHP does but it does not delete the temporary files when the request has been processed. An attacker could fill the Lambda instance disk by performing multiple MultiPart requests containing files. This vulnerability is patched in 2.1.13.
CWE description: The product does not properly control the allocation and maintenance of a limited resource, thereby enabling an actor to influence the amount of resources consumed, eventually leading to the exhaustion of available resources.
CVSS v3 base score: 6.5
Published: 2024-02-01T16:17Z
Repository: https://github.com/brefphp/bref
Language: PHP
Files changed: src/Event/Http/Psr7Bridge.php, tests/Event/Http/CommonHttpTest.php, tests/Event/Http/HttpRequestEventTest.php, tests/Event/Http/Psr7BridgeTest.php
Commit date: 2024-01-31 15:00:21 +0100
Commit message: Support malformed multipart body

For example body containing broken array keys like `key0[key1][key2][`

Diff: diff --git a/src/Event/Http/Psr7Bridge.php b/src/Event/Http/Psr7Bridge.php
index 66728201..7694fdfd 100644
--- a/src/Event/Http/Psr7Bridge.php
+++ b/src/Event/Http/Psr7Bridge.php
@@ -91,40 +91,53 @@ final class Psr7Bridge
         $body = $response->getBody()->getContents();
 
         return new HttpResponse($body, $response->getHeaders(), $response->getStatusCode());
     }
 
+    /**
+     * @return array{0: array<string, UploadedFile>, 1: array<string, mixed>|null}
+     */
     private static function parseBodyAndUploadedFiles(HttpRequestEvent $event): array
     {
-        $bodyString = $event->getBody();
-        $files = [];
-        $parsedBody = null;
         $contentType = $event->getContentType();
-        if ($contentType !== null && $event->getMethod() === 'POST') {
-            if (str_starts_with($contentType, 'application/x-www-form-urlencoded')) {
-                parse_str($bodyString, $parsedBody);
-            } else {
-                $document = new Part("Content-type: $contentType\r\n\r\n" . $bodyString);
-                if ($document->isMultiPart()) {
-                    $parsedBody = [];
-                    foreach ($document->getParts() as $part) {
-                        if ($part->isFile()) {
+        if ($contentType === null || $event->getMethod() !== 'POST') {
+            return [[], null];
+        }
+
+        if (str_starts_with($contentType, 'application/x-www-form-urlencoded')) {
+            $parsedBody = [];
+            parse_str($event->getBody(), $parsedBody);
+            return [[], $parsedBody];
+        }
+
+        // Parse the body as multipart/form-data
+        $document = new Part("Content-type: $contentType\r\n\r\n" . $event->getBody());
+        if (!$document->isMultiPart()) {
+            return [[], null];
+        }
+        $files = [];
+        $queryString = '';
+        foreach ($document->getParts() as $part) {
+            if ($part->isFile()) {
                             $tmpPath = tempnam(sys_get_temp_dir(), self::UPLOADED_FILES_PREFIX);
-                            if ($tmpPath === false) {
-                                throw new RuntimeException('Unable to create a temporary directory');
-                            }
-                            file_put_contents($tmpPath, $part->getBody());
-                            $file = new UploadedFile($tmpPath, filesize($tmpPath), UPLOAD_ERR_OK, $part->getFileName(), $part->getMimeType());
-
-                            self::parseKeyAndInsertValueInArray($files, $part->getName(), $file);
-                        } else {
-                            self::parseKeyAndInsertValueInArray($parsedBody, $part->getName(), $part->getBody());
-                        }
-                    }
+                if ($tmpPath === false) {
+                    throw new RuntimeException('Unable to create a temporary directory');
                 }
+                file_put_contents($tmpPath, $part->getBody());
+                $file = new UploadedFile($tmpPath, filesize($tmpPath), UPLOAD_ERR_OK, $part->getFileName(), $part->getMimeType());
+                self::parseKeyAndInsertValueInArray($files, $part->getName(), $file);
+            } else {
+                // Temporarily store as a query string so that we can use PHP's native parse_str function to parse keys
+                $queryString .= urlencode($part->getName()) . '=' . urlencode($part->getBody()) . '&';
             }
         }
+        if ($queryString !== '') {
+            $parsedBody = [];
+            parse_str($queryString, $parsedBody);
+        } else {
+            $parsedBody = null;
+        }
         return [$files, $parsedBody];
     }
 
     /**
      * Parse a string key like "files[id_cards][jpg][]" and do $array['files']['id_cards']['jpg'][] = $value
diff --git a/tests/Event/Http/CommonHttpTest.php b/tests/Event/Http/CommonHttpTest.php
index 63fd9b4e..73765363 100644
--- a/tests/Event/Http/CommonHttpTest.php
+++ b/tests/Event/Http/CommonHttpTest.php
@@ -399,11 +399,11 @@ Year,Make,Model
 2000,Mercury,Cougar
 \r
 --testBoundary--\r
 ";
         $this->assertBody($body);
-        $this->assertParsedBody([]);
+        $this->assertParsedBody(null);
         $this->assertUploadedFile(
             'foo',
             'lorem.txt',
             'text/plain',
             0,
@@ -552,11 +552,11 @@ Year,Make,Model
 
     abstract protected function assertUri(string $expected): void;
 
     abstract protected function assertHasMultiHeader(bool $expected): void;
 
-    abstract protected function assertParsedBody(array $expected): void;
+    abstract protected function assertParsedBody(array|null $expected): void;
 
     abstract protected function assertSourceIp(string $expected): void;
 
     abstract protected function assertBasicAuthUser(string $expected): void;
 
diff --git a/tests/Event/Http/HttpRequestEventTest.php b/tests/Event/Http/HttpRequestEventTest.php
index d1f2f516..74259e12 100644
--- a/tests/Event/Http/HttpRequestEventTest.php
+++ b/tests/Event/Http/HttpRequestEventTest.php
@@ -110,11 +110,11 @@ class HttpRequestEventTest extends CommonHttpTest
     protected function assertSourceIp(string $expected): void
     {
         $this->assertEquals($expected, $this->event->getSourceIp());
     }
 
-    protected function assertParsedBody(array $expected): void
+    protected function assertParsedBody(array|null $expected): void
     {
         // Not applicable here since the class doesn't parse the body
     }
 
     protected function assertUploadedFile(
diff --git a/tests/Event/Http/Psr7BridgeTest.php b/tests/Event/Http/Psr7BridgeTest.php
index e2ab8de1..d60a69eb 100644
--- a/tests/Event/Http/Psr7BridgeTest.php
+++ b/tests/Event/Http/Psr7BridgeTest.php
@@ -121,11 +121,11 @@ class Psr7BridgeTest extends CommonHttpTest
     protected function assertHasMultiHeader(bool $expected): void
     {
         // Not applicable here
     }
 
-    protected function assertParsedBody(array $expected): void
+    protected function assertParsedBody(array|null $expected): void
     {
         $this->assertEquals($expected, $this->request->getParsedBody());
     }
 
     protected function assertUploadedFile(

Vulnerable code:         $bodyString = $event->getBody();
        $files = [];
        $parsedBody = null;
        if ($contentType !== null && $event->getMethod() === 'POST') {
            if (str_starts_with($contentType, 'application/x-www-form-urlencoded')) {
                parse_str($bodyString, $parsedBody);
            } else {
                $document = new Part("Content-type: $contentType\r\n\r\n" . $bodyString);
                if ($document->isMultiPart()) {
                    $parsedBody = [];
                    foreach ($document->getParts() as $part) {
                        if ($part->isFile()) {
                            if ($tmpPath === false) {
                                throw new RuntimeException('Unable to create a temporary directory');
                            }
                            file_put_contents($tmpPath, $part->getBody());
                            $file = new UploadedFile($tmpPath, filesize($tmpPath), UPLOAD_ERR_OK, $part->getFileName(), $part->getMimeType());

                            self::parseKeyAndInsertValueInArray($files, $part->getName(), $file);
                        } else {
                            self::parseKeyAndInsertValueInArray($parsedBody, $part->getName(), $part->getBody());
                        }
                    }
        $this->assertParsedBody([]);
    abstract protected function assertParsedBody(array $expected): void;
    protected function assertParsedBody(array $expected): void
    protected function assertParsedBody(array $expected): void
Fixed code:     /**
     * @return array{0: array<string, UploadedFile>, 1: array<string, mixed>|null}
     */
        if ($contentType === null || $event->getMethod() !== 'POST') {
            return [[], null];
        }

        if (str_starts_with($contentType, 'application/x-www-form-urlencoded')) {
            $parsedBody = [];
            parse_str($event->getBody(), $parsedBody);
            return [[], $parsedBody];
        }

        // Parse the body as multipart/form-data
        $document = new Part("Content-type: $contentType\r\n\r\n" . $event->getBody());
        if (!$document->isMultiPart()) {
            return [[], null];
        }
        $files = [];
        $queryString = '';
        foreach ($document->getParts() as $part) {
            if ($part->isFile()) {
                if ($tmpPath === false) {
                    throw new RuntimeException('Unable to create a temporary directory');
                file_put_contents($tmpPath, $part->getBody());
                $file = new UploadedFile($tmpPath, filesize($tmpPath), UPLOAD_ERR_OK, $part->getFileName(), $part->getMimeType());
                self::parseKeyAndInsertValueInArray($files, $part->getName(), $file);
            } else {
                // Temporarily store as a query string so that we can use PHP's native parse_str function to parse keys
                $queryString .= urlencode($part->getName()) . '=' . urlencode($part->getBody()) . '&';
        if ($queryString !== '') {
            $parsedBody = [];
            parse_str($queryString, $parsedBody);
        } else {
            $parsedBody = null;
        }
        $this->assertParsedBody(null);
    abstract protected function assertParsedBody(array|null $expected): void;
    protected function assertParsedBody(array|null $expected): void
    protected function assertParsedBody(array|null $expected): void