Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request #7853 from bolt/fix/csrf-check-on-preview
Check CSRF on Preview page
  • Loading branch information
bobdenotter committed May 7, 2020
2 parents 693f1e4 + aba99d2 commit b42cbfc
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/Config.php
Expand Up @@ -311,7 +311,7 @@ protected function parseGeneral()
}

// To remove unacceptable / unwanted extensions from the list of Acceptable File Types
$removeFromAllowedFileTypes = explode('|', 'sh|asp|cgi|php|php3|ph3|php4|ph4|php5|ph5|phtm|phtml');
$removeFromAllowedFileTypes = explode('|', 'sh|asp|cgi|php|php3|ph3|php4|ph4|php5|ph5|phtm|phtml|exe');

// Create a bag with lowercased extensions
$bag = Bag::from($general['accept_file_types']);
Expand Down
4 changes: 3 additions & 1 deletion src/Controller/Async/FilesystemManager.php
Expand Up @@ -428,6 +428,7 @@ public function renameFolder(Request $request)
private function isExtensionChangedAndIsChangeAllowed($oldName, $newName)
{
$user = $this->getUser();

if ($this->users()->hasRole($user['id'], 'root') || $this->users()->hasRole($user['id'], 'admin')) {
return true;
}
Expand Down Expand Up @@ -465,11 +466,12 @@ private function validateFileExtension($filename)
if ($filename[0] === '.') {
return false;
}

// only whitelisted extensions
$extension = pathinfo($filename, PATHINFO_EXTENSION);
$allowedExtensions = $this->getAllowedUploadExtensions();

return $extension === '' || in_array(mb_strtolower($extension), $allowedExtensions);
return in_array(mb_strtolower($extension), $allowedExtensions);
}

/**
Expand Down
7 changes: 7 additions & 0 deletions src/Controller/Frontend.php
Expand Up @@ -17,6 +17,7 @@
use Bolt\Storage\Repository\TaxonomyRepository;
use Bolt\Translation\Translator as Trans;
use Silex\ControllerCollection;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
Expand Down Expand Up @@ -203,6 +204,12 @@ public function preview(Request $request, $contenttypeslug)
throw new MethodNotAllowedHttpException(['POST'], 'This route only accepts POST requests.');
}

// Only accept requests with a valid token
$tokenValue = $request->request->get('content_edit', ['_token' => null])['_token'];
if (!$this->isAllowed('dashboard') || !$this->isCsrfTokenValid($tokenValue, 'content_edit')) {
return new Response('Not allowed or invalid CSRF token', Response::HTTP_FORBIDDEN);
}

$contenttype = $this->getContentType($contenttypeslug);
$formValues = $request->request->all();

Expand Down
24 changes: 21 additions & 3 deletions tests/phpunit/unit/Controller/Async/FilesystemManagerTest.php
Expand Up @@ -25,9 +25,10 @@ class FilesystemManagerTest extends ControllerUnitTest
{
const FILESYSTEM = 'files';

const FILE_NAME = '__phpunit_test_file_delete_me';
const FILE_NAME = '__phpunit_test_file_delete_me.txt';
const FILE_NAME_NOT_ALLOWED = '__phpunit_test_file_delete_me.exe';
const FILE_NAME_2 = '__phpunit_test_file_2_delete_me';
const FILE_NAME_NOT_ALLOWED_2 = '__phpunit_test_file_delete_me';
const FILE_NAME_2 = '__phpunit_test_file_2_delete_me.txt';
const FOLDER_NAME = '__phpunit_test_folder_delete_me';
const FOLDER_NAME_2 = '__phpunit_test_folder_2_delete_me';

Expand Down Expand Up @@ -153,6 +154,23 @@ public function testCreateFileInvalidExtension()
$this->assertFalse($this->getService('filesystem')->has(self::FILESYSTEM . '://' . self::FILE_NAME_NOT_ALLOWED));
}

public function testCreateFileInvalidExtension2()
{
$this->setRequest(Request::create('/async/file/create', 'POST', [
'namespace' => self::FILESYSTEM,
'parentPath' => '',
'filename' => self::FILE_NAME_NOT_ALLOWED_2,
'token' => $this->token,
]));
$response = $this->controller()->createFile($this->getRequest());

$this->assertInstanceOf(JsonResponse::class, $response);
$this->assertEquals(Response::HTTP_BAD_REQUEST, $response->getStatusCode());

// Test whether the new file is not saved
$this->assertFalse($this->getService('filesystem')->has(self::FILESYSTEM . '://' . self::FILE_NAME_NOT_ALLOWED));
}

/**
* Duplicating a file five times should create FILENAME_copy1-5.EXT. This should work for both regular filenames
* and dotfiles.
Expand Down Expand Up @@ -255,7 +273,7 @@ public function testInvalidRename()
* Object doesn't exist
*/
$this->createObject($object, $data['old']);
$response = $this->renameObject($object, $data['old'] . '_nonexistent', $data['new']);
$response = $this->renameObject($object, $data['old'] . '_nonexistent.txt', $data['new']);

$this->assertInstanceOf(JsonResponse::class, $response);
$this->assertEquals(Response::HTTP_NOT_FOUND, $response->getStatusCode());
Expand Down
3 changes: 1 addition & 2 deletions tests/phpunit/unit/Controller/FrontendTest.php
Expand Up @@ -328,8 +328,7 @@ public function testPreview()

$response = $this->controller()->preview($this->getRequest(), 'pages');

$this->assertTrue($response instanceof TemplateView);
$this->assertSame('record.twig', $response->getTemplate());
$this->assertFalse($response instanceof TemplateView);
}

public function testListing()
Expand Down

0 comments on commit b42cbfc

Please sign in to comment.