tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

[CVE-2020-26138] Validate custom multi-file uploads

Browse Source
This commit is contained in:
Steve Boyd 2021-02-19 10:28:03 +13:00
parent 3bb435c241
commit 06dbd5237b
2 changed files with 98 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/Forms/FileField.php
View File

@ -177,13 +177,46 @@ class FileField extends FormField implements FileHandleField
public function validate($validator)
{
if (!isset($_FILES[$this->name])) {
// FileField with the name multi_file_syntax[] or multi_file_syntax[key] will have the brackets trimmed in
// $_FILES super-global so it will be stored as $_FILES['mutli_file_syntax']
// multi-file uploads, which are not officially supported by Silverstripe, though may be
// implemented in custom code, so we should still ensure they are at least validated
$isMultiFileUpload = strpos($this->name, '[') !== false;
$fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name);
if (!isset($_FILES[$fieldName])) {
return true;
}
$tmpFile = $_FILES[$this->name];
if ($isMultiFileUpload) {
$isValid = true;
foreach (array_keys($_FILES[$fieldName]['name']) as $key) {
$fileData = [
'name' => $_FILES[$fieldName]['name'][$key],
'type' => $_FILES[$fieldName]['type'][$key],
'tmp_name' => $_FILES[$fieldName]['tmp_name'][$key],
'error' => $_FILES[$fieldName]['error'][$key],
'size' => $_FILES[$fieldName]['size'][$key],
];
if (!$this->validateFileData($validator, $fileData)) {
$isValid = false;
}
}
return $isValid;
}
$valid = $this->upload->validate($tmpFile);
// regular single-file upload
return $this->validateFileData($validator, $_FILES[$this->name]);
}
/**
* @param Validator $validator
* @param array $fileData
* @return bool
*/
private function validateFileData($validator, array $fileData): bool
{
$valid = $this->upload->validate($fileData);
if (!$valid) {
$errors = $this->upload->getErrors();
if ($errors) {
@ -193,7 +226,6 @@ class FileField extends FormField implements FileHandleField
}
return false;
}
return true;
}

62
tests/php/Forms/FileFieldTest.php
View File

@ -3,6 +3,7 @@
namespace SilverStripe\Forms\Tests;
use ReflectionMethod;
use SilverStripe\Assets\Upload_Validator;
use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Control\Controller;
use SilverStripe\Forms\FileField;
@ -39,6 +40,67 @@ class FileFieldTest extends FunctionalTest
$this->assertTrue($form->validationResult()->isValid());
}
/**
* Test that FileField::validate() is run on FileFields with both single and multi-file syntax
* By default FileField::validate() will return true early if the $_FILES super-global does not contain the
* corresponding FileField::name. This early return means the files was not fully run through FileField::validate()
* So for this test we create an invalid file upload on purpose and test that false was returned which means that
* the file was run through FileField::validate() function
*/
public function testMultiFileSyntaxUploadIsValidated()
{
$names = [
'single_file_syntax',
'multi_file_syntax_a[]',
'multi_file_syntax_b[0]',
'multi_file_syntax_c[key]'
];
foreach ($names as $name) {
$form = new Form(
Controller::curr(),
'Form',
new FieldList($fileField = new FileField($name, 'My desc')),
new FieldList()
);
$fileData = $this->createInvalidUploadedFileData($name, "FileFieldTest.txt");
// FileFields with multi_file_syntax[] files will appear in the $_FILES super-global
// with the [] brackets trimmed e.g. $_FILES[multi_file_syntax]
$_FILES = [preg_replace('#\[(.*?)\]#', '', $name) => $fileData];
$fileField->setValue($fileData);
$validator = $form->getValidator();
$isValid = $fileField->validate($validator);
$this->assertFalse($isValid, "$name was run through the validate() function");
}
}
protected function createInvalidUploadedFileData($name, $tmpFileName): array
{
$tmpFilePath = TEMP_PATH . DIRECTORY_SEPARATOR . $tmpFileName;
// multi_file_syntax
if (strpos($name, '[') !== false) {
$key = 0;
if (preg_match('#\[(.+?)\]#', $name, $m)) {
$key = $m[1];
}
return [
'name' => [$key => $tmpFileName],
'type' => [$key => 'text/plaintext'],
'size' => [$key => 0],
'tmp_name' => [$key => $tmpFilePath],
'error' => [$key => UPLOAD_ERR_NO_FILE],
];
}
// single_file_syntax
return [
'name' => $tmpFileName,
'type' => 'text/plaintext',
'size' => 0,
'tmp_name' => $tmpFilePath,
'error' => UPLOAD_ERR_NO_FILE,
];
}
/**
* @skipUpgrade
*/