Merge pull request #48 from weierophinney/security/stream-destruction

Security tightening: verify a stream file name is a string before unlinking
laminas · Jan 5, 2021 · 019f31f · 019f31f
2 parents 25d7cf0 + eab608e
commit 019f31f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 1 deletion.
diff --git a/src/Response/Stream.php b/src/Response/Stream.php
@@ -287,7 +287,7 @@ public function __destruct()
         if (is_resource($this->stream)) {
             $this->stream = null; //Could be listened by others
         }
-        if ($this->cleanup) {
+        if ($this->cleanup && is_string($this->streamName) && file_exists($this->streamName)) {
             ErrorHandler::start(E_WARNING);
             unlink($this->streamName);
             ErrorHandler::stop();

diff --git a/test/Response/ResponseStreamTest.php b/test/Response/ResponseStreamTest.php
@@ -13,6 +13,21 @@
 
 class ResponseStreamTest extends TestCase
 {
+    /** @var null|string */
+    private $tempFile;
+
+    public function setUp(): void
+    {
+        $this->tempFile = null;
+    }
+
+    public function tearDown(): void
+    {
+        if (null !== $this->tempFile && file_exists($this->tempFile)) {
+            unlink($this->tempFile);
+        }
+    }
+
     public function testResponseFactoryFromStringCreatesValidResponse()
     {
         $string = 'HTTP/1.0 200 OK' . "\r\n\r\n" . 'Foo Bar' . "\r\n";
@@ -78,6 +93,35 @@ public function test300isRedirect()
         $this->assertFalse($response->isSuccess(), 'Response is an error, but isSuccess() returned true');
     }
 
+    /**
+     * @see https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-3007
+     */
+    public function testDestructionDoesNothingIfStreamIsNotAResourceAndStreamNameIsNotAString(): void
+    {
+        $this->tempFile = tempnam(sys_get_temp_dir(), 'lhrs');
+        $streamObject = new class($this->tempFile) {
+            private $tempFile;
+
+            public function __construct(string $tempFile)
+            {
+                $this->tempFile = $tempFile;
+            }
+
+            public function __toString()
+            {
+                return $this->tempFile;
+            }
+        };
+
+        $response = new Stream();
+        $response->setCleanup(true);
+        $response->setStreamName($streamObject);
+
+        unset($response);
+
+        $this->assertFileExists($this->tempFile);
+    }
+
     /**
      * Helper function: read test response from file
      *