[ss-2015-027] FIX HtmlEditorField_Toolbar#viewfile not whitelisting URLs

Fixes #4947
This commit is contained in:
Damian Mooyman 2016-05-03 14:16:05 +12:00
parent 6948267c41
commit 70480f5ee4
3 changed files with 220 additions and 18 deletions

View File

@ -413,37 +413,120 @@ class HtmlEditorField_Toolbar extends RequestHandler {
return $form; return $form;
} }
/**
* List of allowed schemes (no wildcard, all lower case) or empty to allow all schemes
*
* @config
* @var array
*/
private static $fileurl_scheme_whitelist = array('http', 'https');
/**
* List of allowed domains (no wildcard, all lower case) or empty to allow all domains
*
* @config
* @var array
*/
private static $fileurl_domain_whitelist = array();
/**
* Find local File dataobject given ID
*
* @param int $id
* @return array
*/
protected function viewfile_getLocalFileByID($id) {
/** @var File $file */
$file = DataObject::get_by_id('File', $id);
if ($file && $file->canView()) {
return array($file, $file->getURL());
}
return [null, null];
}
/**
* Get remote File given url
*
* @param string $fileUrl Absolute URL
* @return array
* @throws SS_HTTPResponse_Exception
*/
protected function viewfile_getRemoteFileByURL($fileUrl) {
if(!Director::is_absolute_url($fileUrl)) {
throw $this->getErrorFor(_t(
"HtmlEditorField_Toolbar.ERROR_ABSOLUTE",
"Only absolute urls can be embedded"
));
}
$scheme = strtolower(parse_url($fileUrl, PHP_URL_SCHEME));
$allowed_schemes = self::config()->fileurl_scheme_whitelist;
if (!$scheme || ($allowed_schemes && !in_array($scheme, $allowed_schemes))) {
throw $this->getErrorFor(_t(
"HtmlEditorField_Toolbar.ERROR_SCHEME",
"This file scheme is not included in the whitelist"
));
}
$domain = strtolower(parse_url($fileUrl, PHP_URL_HOST));
$allowed_domains = self::config()->fileurl_domain_whitelist;
if (!$domain || ($allowed_domains && !in_array($domain, $allowed_domains))) {
throw $this->getErrorFor(_t(
"HtmlEditorField_Toolbar.ERROR_HOSTNAME",
"This file hostname is not included in the whitelist"
));
}
return [null, $fileUrl];
}
/**
* Prepare error for the front end
*
* @param string $message
* @param int $code
* @return SS_HTTPResponse_Exception
*/
protected function getErrorFor($message, $code = 400) {
$exception = new SS_HTTPResponse_Exception($message, $code);
$exception->getResponse()->addHeader('X-Status', $message);
return $exception;
}
/** /**
* View of a single file, either on the filesystem or on the web. * View of a single file, either on the filesystem or on the web.
* *
* @throws SS_HTTPResponse_Exception
* @param SS_HTTPRequest $request * @param SS_HTTPRequest $request
* @return string * @return string
*/ */
public function viewfile($request) { public function viewfile($request) {
// TODO Would be cleaner to consistently pass URL for both local and remote files,
// but GridField doesn't allow for this kind of metadata customization at the moment.
$file = null; $file = null;
if($url = $request->getVar('FileURL')) { $url = null;
// URLS should be used for remote resources (not local assets) // Get file and url by request method
$url = Director::absoluteURL($url); if($fileUrl = $request->getVar('FileURL')) {
// Get remote url
list($file, $url) = $this->viewfile_getRemoteFileByURL($fileUrl);
} elseif($id = $request->getVar('ID')) { } elseif($id = $request->getVar('ID')) {
// Use local dataobject // Or we could have been passed an ID directly
$file = DataObject::get_by_id('File', $id); list($file, $url) = $this->viewfile_getLocalFileByID($id);
if(!$file) {
throw new InvalidArgumentException("File could not be found");
}
$url = $file->getURL();
if(!$url) {
return $this->httpError(404, 'File not found');
}
} else { } else {
throw new LogicException('Need either "ID" or "FileURL" parameter to identify the file'); // Or we could have been passed nothing, in which case panic
throw $this->getErrorFor(_t(
"HtmlEditorField_Toolbar.ERROR_ID",
'Need either "ID" or "FileURL" parameter to identify the file'
));
}
// Validate file exists
if(!$url) {
throw $this->getErrorFor(_t(
"HtmlEditorField_Toolbar.ERROR_NOTFOUND",
'Unable to find file to view'
));
} }
// Instanciate file wrapper and get fields based on its type // Instanciate file wrapper and get fields based on its type
// Check if appCategory is an image and exists on the local system, otherwise use oEmbed to refference a // Check if appCategory is an image and exists on the local system, otherwise use oEmbed to refference a
// remote image // remote image
$fileCategory = File::get_app_category(File::get_file_extension($url)); $fileCategory = $this->getFileCategory($url, $file);
switch($fileCategory) { switch($fileCategory) {
case 'image': case 'image':
case 'image/supported': case 'image/supported':
@ -456,10 +539,12 @@ class HtmlEditorField_Toolbar extends RequestHandler {
// Only remote files can be linked via o-embed // Only remote files can be linked via o-embed
// {@see HtmlEditorField_Toolbar::getAllowedExtensions()) // {@see HtmlEditorField_Toolbar::getAllowedExtensions())
if($file) { if($file) {
throw new InvalidArgumentException( throw $this->getErrorFor(_t(
"HtmlEditorField_Toolbar.ERROR_OEMBED_REMOTE",
"Oembed is only compatible with remote files" "Oembed is only compatible with remote files"
); ));
} }
// Other files should fallback to oembed // Other files should fallback to oembed
$fileWrapper = new HtmlEditorField_Embed($url, $file); $fileWrapper = new HtmlEditorField_Embed($url, $file);
break; break;
@ -472,10 +557,28 @@ class HtmlEditorField_Toolbar extends RequestHandler {
))->renderWith($this->templateViewFile); ))->renderWith($this->templateViewFile);
} }
/**
* Guess file category from either a file or url
*
* @param string $url
* @param File $file
* @return string
*/
protected function getFileCategory($url, $file) {
if($file) {
return $file->appCategory();
}
if($url) {
return File::get_app_category(File::get_file_extension($url));
}
return null;
}
/** /**
* Find all anchors available on the given page. * Find all anchors available on the given page.
* *
* @return array * @return array
* @throws SS_HTTPResponse_Exception
*/ */
public function getanchors() { public function getanchors() {
$id = (int)$this->getRequest()->getVar('PageID'); $id = (int)$this->getRequest()->getVar('PageID');

View File

@ -0,0 +1,81 @@
<?php
class HtmlEditorFieldToolbarTest_Toolbar extends HtmlEditorField_Toolbar {
public function viewfile_getLocalFileByID($id) {
return parent::viewfile_getLocalFileByID($id);
}
public function viewfile_getRemoteFileByURL($fileUrl) {
return parent::viewfile_getRemoteFileByURL($fileUrl);
}
}
class HtmlEditorFieldToolbarTest extends SapphireTest {
protected static $fixture_file = 'HtmlEditorFieldToolbarTest.yml';
/**
* @return HtmlEditorFieldToolbarTest_Toolbar
*/
protected function getToolbar() {
return new HtmlEditorFieldToolbarTest_Toolbar(null, '/');
}
public function setUp() {
parent::setUp();
Config::inst()->update('HtmlEditorField_Toolbar', 'fileurl_scheme_whitelist', array('http'));
Config::inst()->update('HtmlEditorField_Toolbar', 'fileurl_domain_whitelist', array('example.com'));
// Filesystem mock
AssetStoreTest_SpyStore::activate(__CLASS__);
// Load up files
/** @var File $file1 */
$file1 = $this->objFromFixture('File', 'example_file');
$file1->setFromString(str_repeat('x', 1000), $file1->Name);
$file1->write();
/** @var Image $image1 */
$image1 = $this->objFromFixture('Image', 'example_image');
$image1->setFromLocalFile(
__DIR__ . '/images/HTMLEditorFieldTest-example.jpg',
'folder/subfolder/HTMLEditorFieldTest_example.jpg'
);
$image1->write();
}
public function testValidLocalReference() {
/** @var File $exampleFile */
$exampleFile = $this->objFromFixture('File', 'example_file');
$expectedUrl = $exampleFile->AbsoluteLink();
Config::inst()->update('HtmlEditorField_Toolbar', 'fileurl_domain_whitelist', array(
'example.com',
strtolower(parse_url($expectedUrl, PHP_URL_HOST))
));
list($file, $url) = $this->getToolbar()->viewfile_getRemoteFileByURL($exampleFile->AbsoluteLink());
$this->assertEquals($expectedUrl, $url);
}
public function testValidScheme() {
list($file, $url) = $this->getToolbar()->viewfile_getRemoteFileByURL('http://example.com/test.pdf');
$this->assertEquals($url, 'http://example.com/test.pdf');
}
/** @expectedException SS_HTTPResponse_Exception */
public function testInvalidScheme() {
list($file, $url) = $this->getToolbar()->viewfile_getRemoteFileByURL('nosuchscheme://example.com/test.pdf');
}
public function testValidDomain() {
list($file, $url) = $this->getToolbar()->viewfile_getRemoteFileByURL('http://example.com/test.pdf');
$this->assertEquals($url, 'http://example.com/test.pdf');
}
/** @expectedException SS_HTTPResponse_Exception */
public function testInvalidDomain() {
list($file, $url) = $this->getToolbar()->viewfile_getRemoteFileByURL('http://evil.com/test.pdf');
}
}

View File

@ -0,0 +1,18 @@
Folder:
folder1:
Name: folder
Title: folder
folder2:
Name: subfolder
Title: subfolder
Parent: =>Folder.folder1
File:
example_file:
Name: example.pdf
Parent: =>Folder.folder2
Image:
example_image:
Name: HTMLEditorFieldTest_example.jpg
Parent: =>Folder.folder2