Skip to content

Commit

Permalink
Security - Fix joomla/cms-security#522 (#32)
Browse files Browse the repository at this point in the history
Make sure that unpacked files stay below target directory
  • Loading branch information
nibra authored Mar 29, 2022
1 parent 92110ce commit cedda2c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 7 deletions.
27 changes: 24 additions & 3 deletions src/Tar.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,8 @@ public function __construct($options = [])
*/
public function extract($archive, $destination)
{
$this->data = null;
$this->metadata = null;

$this->data = file_get_contents($archive);
$this->data = file_get_contents($archive);

if (!$this->data)
{
Expand All @@ -121,6 +119,11 @@ public function extract($archive, $destination)
$buffer = $this->metadata[$i]['data'];
$path = Path::clean($destination . '/' . $this->metadata[$i]['name']);

if (!$this->isBelow($destination, $destination . '/' . $this->metadata[$i]['name']))
{
throw new \OutOfBoundsException('Unable to write outside of destination path', 100);
}

// Make sure the destination folder exists
if (!Folder::create(\dirname($path)))
{
Expand Down Expand Up @@ -244,4 +247,22 @@ protected function getTarInfo(&$data)

$this->metadata = $returnArray;
}

/**
* Check if a path is below a given destination path
*
* @param string $destination The destination path
* @param string $path The path to be checked
*
* @return boolean
*
* @since 2.0.1
*/
private function isBelow($destination, $path): bool
{
$absoluteRoot = Path::clean(Path::resolve($destination));
$absolutePath = Path::clean(Path::resolve($path));

return strpos($absolutePath, $absoluteRoot) === 0;
}
}
8 changes: 4 additions & 4 deletions src/Zip.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,8 @@ public function checkZipData($data)
*/
protected function extractCustom($archive, $destination)
{
$this->data = null;
$this->metadata = null;

$this->data = file_get_contents($archive);
$this->data = file_get_contents($archive);

if (!$this->data)
{
Expand All @@ -248,7 +246,7 @@ protected function extractCustom($archive, $destination)

if (!$this->isBelow($destination, $destination . '/' . $metadata['name']))
{
throw new \RuntimeException('Unable to write outside of destination path', 100);
throw new \OutOfBoundsException('Unable to write outside of destination path', 100);
}

// Make sure the destination folder exists
Expand Down Expand Up @@ -681,6 +679,8 @@ private function createZipFile(array $contents, array $ctrlDir, string $path): b
* @param string $path The path to be checked
*
* @return boolean
*
* @since 1.1.10
*/
private function isBelow($destination, $path): bool
{
Expand Down

0 comments on commit cedda2c

Please sign in to comment.