diff --git a/system/HTTP/Files/UploadedFile.php b/system/HTTP/Files/UploadedFile.php index b43923026021..73e68dad1fd7 100644 --- a/system/HTTP/Files/UploadedFile.php +++ b/system/HTTP/Files/UploadedFile.php @@ -142,7 +142,14 @@ public function move(string $targetPath, ?string $name = null, bool $overwrite = throw HTTPException::forInvalidFile(); } + $hasClientName = $name === null; $name ??= $this->getName(); + + if ($hasClientName) { + helper('security'); + $name = sanitize_filename($name); + } + $destination = $overwrite ? $targetPath . $name : $this->getDestination($targetPath . $name); try { diff --git a/tests/system/HTTP/Files/FileMovingTest.php b/tests/system/HTTP/Files/FileMovingTest.php index 45a9fd179837..e9f5837c698a 100644 --- a/tests/system/HTTP/Files/FileMovingTest.php +++ b/tests/system/HTTP/Files/FileMovingTest.php @@ -100,6 +100,37 @@ public function testMove(): void $this->assertTrue($this->root->hasChild('destination/' . $finalFilename . '_1.txt')); } + public function testMovePathTraversal(): void + { + service('superglobals')->setFilesArray([ + 'userfile1' => [ + 'name' => '../../public/shell.php', + 'type' => 'text/plain', + 'size' => 124, + 'tmp_name' => '/tmp/fileA.php', + 'error' => 0, + ], + ]); + + $collection = new FileCollection(); + + $this->assertTrue($collection->hasFile('userfile1')); + + $destination = $this->destination; + // Create the destination if not exists + if (! is_dir($destination)) { + mkdir($destination, 0777, true); + } + + $file = $collection->getFile('userfile1'); + $this->assertInstanceOf(UploadedFile::class, $file); + + $file->move($destination); + + $this->assertTrue($this->root->hasChild('destination/publicshell.php')); + $this->assertFalse($this->root->hasChild('public/shell.php')); + } + public function testMoveOverwriting(): void { $finalFilename = 'file_with_delimiters_underscore';