Skip to content

Commit

Permalink
Fix remote file overwrite and arbitrary file injection in the file sy…
Browse files Browse the repository at this point in the history
…stem
  • Loading branch information
williamdes committed Dec 17, 2023
1 parent 408f54d commit d3552ef
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 14 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
"require": {
"php": ">=8.1.0",
"cakephp/filesystem": "^3.0",
"monolog/monolog": "^2.0"
"monolog/monolog": "^2.0",
"ondrej-vrto/php-filename-sanitize": "^1.4"
},
"require-dev": {
"phpunit/phpunit": "~10.0"
Expand Down
23 changes: 10 additions & 13 deletions src/Resumable.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Dilab\Network\Response;
use Monolog\Logger;
use Monolog\Handler\StreamHandler;
use OndrejVrto\FilenameSanitize\FilenameSanitize;

class Resumable
{
Expand Down Expand Up @@ -159,18 +160,14 @@ public function getExtension()
}

/**
* Makes sure the orginal extension never gets overriden by user defined filename.
* Creates a safe name
*
* @param string User defined filename
* @param string Original filename
* @return string Filename that always has an extension from the original file
* @param string $name Original name
* @return string A safer name
*/
private function createSafeFilename($filename, $originalFilename)
private function createSafeName(string $name): string
{
$filename = $this->removeExtension($filename);
$extension = $this->findExtension($originalFilename);

return sprintf('%s.%s', $filename, $extension);
return FilenameSanitize::of($name)->get();
}

public function handleTestChunk()
Expand Down Expand Up @@ -227,9 +224,9 @@ private function createFileAndDeleteTmp($identifier, $filename)

// if the user has set a custom filename
if (null !== $this->filename) {
$finalFilename = $this->createSafeFilename($this->filename, $filename);
$finalFilename = $this->createSafeName($this->filename);
} else {
$finalFilename = $filename;
$finalFilename = $this->createSafeName($filename);
}

// replace filename reference by the final file
Expand Down Expand Up @@ -288,7 +285,7 @@ public function tmpChunkDir($identifier)
if (!empty($this->instanceId)){
$tmpChunkDir .= $this->instanceId . DIRECTORY_SEPARATOR;
}
$tmpChunkDir .= $identifier;
$tmpChunkDir .= $this->createSafeName($identifier);
$this->ensureDirExists($tmpChunkDir);
return $tmpChunkDir;
}
Expand Down Expand Up @@ -318,7 +315,7 @@ private function ensureDirExists($path)

public function tmpChunkFilename($filename, $chunkNumber)
{
return $filename . '.' . str_pad($chunkNumber, 4, 0, STR_PAD_LEFT);
return $this->createSafeName($filename) . '.' . str_pad($chunkNumber, 4, 0, STR_PAD_LEFT);
}

public function getExclusiveFileHandle($name)
Expand Down

0 comments on commit d3552ef

Please sign in to comment.