Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions _build/test/Tests/Processors/Browser/BrowserSanitizeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php

/*
* This file is part of the MODX Revolution package.
*
* Copyright (c) MODX, LLC
*
* For complete copyright and license information, see the COPYRIGHT and LICENSE
* files found in the top-level directory of this distribution.
*
* @package modx-test
*/

namespace MODX\Revolution\Tests\Processors\Browser;

use MODX\Revolution\MODxTestCase;
use MODX\Revolution\Processors\Browser\File\Remove;

/**
* Regression for #16663: Browser::sanitize must keep double dots inside filenames.
*
* @package modx-test
* @subpackage modx
* @group Processors
* @group BrowserProcessors
*/
class BrowserSanitizeTest extends MODxTestCase
{
/** @var Remove */
private $processor;

/**
* @before
*/
public function setUpFixtures()
{
parent::setUpFixtures();
$this->processor = new Remove($this->modx);
}

/**
* @dataProvider providerSanitize
* @param string $input
* @param string $expected
*/
public function testSanitizePreservesFilenameDotsAndTrailingSlash($input, $expected)
{
$this->assertSame($expected, $this->processor->sanitize($input));
}

public function providerSanitize()
{
return [
'double dots in filename' => ['somefile..txt', 'somefile..txt'],
'directory plus double-dot file' => ['folder/somefile..txt', 'folder/somefile..txt'],
'preserves trailing slash for File/Create' => ['folder/', 'folder/'],
'collapses duplicate slashes' => ['folder//file.txt', 'folder/file.txt'],
'hidden file leading dot' => ['.htaccess', '.htaccess'],
'url-encoded filename' => ['some%20file..txt', 'some file..txt'],
'leaves relative parent segment for Flysystem' => ['../escape', '../escape'],
'leaves nested parent segment for Flysystem' => ['a/../b', 'a/../b'],
];
}
}
8 changes: 5 additions & 3 deletions core/src/Revolution/Processors/Browser/Browser.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

/*
* This file is part of the MODX Revolution package.
*
Expand All @@ -10,7 +11,6 @@

namespace MODX\Revolution\Processors\Browser;


use MODX\Revolution\Processors\Processor;
use MODX\Revolution\Sources\modMediaSource;

Expand Down Expand Up @@ -135,7 +135,10 @@ public function handleResponse($response)


/**
* @param $file
* Soft-sanitize a browser path/filename for media-source operations.
* Keeps valid names like somefile..txt. Path traversal is enforced by Flysystem.
*
* @param string $file
*
* @return string
*/
Expand All @@ -146,7 +149,6 @@ public function sanitize($file)
if (strpos($file, '.') === 0) {
$file = preg_replace('/^(\.)([\s\/]*)(.*)/', '$1$3', $file);
}
$file = preg_replace('/\.(?![\w\-\~\s])/u', '', $file);
$file = preg_replace('/\/{2,}/', '/', $file);
$file = strip_tags($file);

Expand Down
Loading