diff --git a/lib/Qubit.class.php b/lib/Qubit.class.php index 00ee0c5f32..7a0a477c94 100644 --- a/lib/Qubit.class.php +++ b/lib/Qubit.class.php @@ -175,14 +175,26 @@ public static function saveTemporaryFile($name, $contents) chmod($tmpDir, 0775); } + $tmpFileName = tempnam($tmpDir, 'QUBIT'); + if (false === $tmpFileName) { + return false; + } + + $extension = ''; $pathInfo = pathinfo($name); - $extension = $pathInfo['extension']; + if (isset($pathInfo['extension']) && preg_match('/^[a-z0-9]+/i', $pathInfo['extension'], $matches)) { + $extension = strtolower($matches[0]); + } + + if ('' !== $extension) { + $tmpFileNameWithExtension = $tmpFileName.'.'.$extension; + if (!rename($tmpFileName, $tmpFileNameWithExtension)) { + @unlink($tmpFileName); + + return false; + } - // Get a unique file name (to avoid clashing file names) - $tmpFileName = null; - while (null == $tmpFileName || file_exists($tmpFileName)) { - $uniqueString = substr(md5(time()), 0, 8); - $tmpFileName = $tmpDir.'/QUBIT'.$uniqueString.'.'.$extension; + $tmpFileName = $tmpFileNameWithExtension; } return false != file_put_contents($tmpFileName, $contents) ? $tmpFileName : false; diff --git a/test/phpunit/lib/QubitTest.php b/test/phpunit/lib/QubitTest.php new file mode 100644 index 0000000000..328c7ddd6b --- /dev/null +++ b/test/phpunit/lib/QubitTest.php @@ -0,0 +1,72 @@ +. + */ + +use PHPUnit\Framework\TestCase; + +/** + * @internal + * + * @covers \Qubit + */ +class QubitTest extends TestCase +{ + private $temporaryFiles = []; + + protected function tearDown(): void + { + foreach ($this->temporaryFiles as $path) { + if (is_file($path)) { + unlink($path); + } + } + } + + public function testSaveTemporaryFilePreservesSafeExtension(): void + { + $path = $this->saveTemporaryFile('document.pdf', 'contents'); + + $this->assertMatchesRegularExpression('/^QUBIT.+\.pdf$/', basename($path)); + $this->assertSame('contents', file_get_contents($path)); + } + + public function testSaveTemporaryFileStripsUnsafeExtensionSuffix(): void + { + $path = $this->saveTemporaryFile('document.pdf;id', 'contents'); + + $this->assertMatchesRegularExpression('/^QUBIT.+\.pdf$/', basename($path)); + $this->assertStringNotContainsString(';', basename($path)); + } + + public function testSaveTemporaryFileAllowsExtensionlessNames(): void + { + $path = $this->saveTemporaryFile('document', 'contents'); + + $this->assertMatchesRegularExpression('/^QUBIT[^.]+$/', basename($path)); + $this->assertSame('contents', file_get_contents($path)); + } + + private function saveTemporaryFile(string $name, string $contents): string + { + $path = Qubit::saveTemporaryFile($name, $contents); + $this->assertNotFalse($path); + $this->temporaryFiles[] = $path; + + return $path; + } +}