Merge pull request #3101 from tractorcow/pulls/3.1-fileexists-checking

BUG Better checking of existing files
This commit is contained in:
Hamish Friedlander 2014-05-06 15:32:06 +12:00
commit 50e1ed2f72
2 changed files with 51 additions and 12 deletions

View File

@ -1262,16 +1262,16 @@ class UploadField extends FileField {
}
/**
* Determines if a specified file exists
* Check if file exists, both checking filtered filename and exact filename
*
* @param SS_HTTPRequest $request
* @param string $originalFile Filename
* @return bool
*/
public function fileexists(SS_HTTPRequest $request) {
protected function checkFileExists($originalFile) {
// Check both original and safely filtered filename
$originalFile = $request->requestVar('filename');
$nameFilter = FileNameFilter::create();
$filteredFile = basename($nameFilter->filter($originalFile));
$filteredFile = $nameFilter->filter($originalFile);
// Resolve expected folder name
$folderName = $this->getFolderName();
@ -1281,17 +1281,32 @@ class UploadField extends FileField {
: ASSETS_PATH."/";
// check if either file exists
$exists = false;
foreach(array($originalFile, $filteredFile) as $file) {
if(file_exists($parentPath.$file)) {
$exists = true;
break;
}
return file_exists($parentPath.$originalFile)
|| file_exists($parentPath.$filteredFile);
}
/**
* Determines if a specified file exists
*
* @param SS_HTTPRequest $request
*/
public function fileexists(SS_HTTPRequest $request) {
// Assert that requested filename doesn't attempt to escape the directory
$originalFile = $request->requestVar('filename');
if($originalFile !== basename($originalFile)) {
$return = array(
'error' => _t('File.NOVALIDUPLOAD', 'File is not a valid upload')
);
} else {
$return = array(
'exists' => $this->checkFileExists($originalFile)
);
}
// Encode and present response
$response = new SS_HTTPResponse(Convert::raw2json(array('exists' => $exists)));
$response = new SS_HTTPResponse(Convert::raw2json($return));
$response->addHeader('Content-Type', 'application/json');
if (!empty($return['error'])) $response->setStatusCode(400);
return $response;
}

View File

@ -680,6 +680,30 @@ class UploadFieldTest extends FunctionalTest {
$responseExistsData = json_decode($responseExists->getBody());
$this->assertFalse($responseExists->isError());
$this->assertTrue($responseExistsData->exists);
// Test that files with invalid characters are rewritten safely and both report exists
// Check that uploaded files can be detected in the root
$tmpFileName = '_test___Upload___Bad.txt';
$tmpFileNameExpected = 'test-Upload-Bad.txt';
$response = $this->mockFileUpload('NoRelationField', $tmpFileName);
$this->assertFalse($response->isError());
$this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileNameExpected");
// With original file
$responseExists = $this->mockFileExists('NoRelationField', $tmpFileName);
$responseExistsData = json_decode($responseExists->getBody());
$this->assertFalse($responseExists->isError());
$this->assertTrue($responseExistsData->exists);
// With rewritten file
$responseExists = $this->mockFileExists('NoRelationField', $tmpFileNameExpected);
$responseExistsData = json_decode($responseExists->getBody());
$this->assertFalse($responseExists->isError());
$this->assertTrue($responseExistsData->exists);
// Test that attempts to navigate outside of the directory return false
$responseExists = $this->mockFileExists('NoRelationField', "../../../../var/private/$tmpFileName");
$responseExistsData = json_decode($responseExists->getBody());
$this->assertTrue($responseExists->isError());
$this->assertContains('File is not a valid upload', $responseExists->getBody());
}
protected function getMockForm() {