Hardened image file validation by removing custom validation

- Added test to check PHP files cannot be uploaded as an image.
This commit is contained in:
Dan Brown 2019-03-20 23:59:55 +00:00
parent 00703fa817
commit 37b91b6b0e
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
6 changed files with 31 additions and 13 deletions

View File

@ -119,7 +119,7 @@ class ImageController extends Controller
{
$this->checkPermission('image-create-all');
$this->validate($request, [
'file' => 'is_image'
'file' => 'mimes:jpeg,png,gif,bmp,webp,tiff'
]);
if (!$this->imageRepo->isValidType($type)) {
@ -135,7 +135,6 @@ class ImageController extends Controller
return response($e->getMessage(), 500);
}
return response()->json($image);
}

View File

@ -21,12 +21,6 @@ class AppServiceProvider extends ServiceProvider
*/
public function boot()
{
// Custom validation methods
Validator::extend('is_image', function ($attribute, $value, $parameters, $validator) {
$imageMimes = ['image/png', 'image/bmp', 'image/gif', 'image/jpeg', 'image/jpg', 'image/tiff', 'image/webp'];
return in_array($value->getMimeType(), $imageMimes);
});
// Custom blade view directives
Blade::directive('icon', function ($expression) {
return "<?php echo icon($expression); ?>";

View File

@ -69,7 +69,6 @@ return [
'timezone' => 'The :attribute must be a valid zone.',
'unique' => 'The :attribute has already been taken.',
'url' => 'The :attribute format is invalid.',
'is_image' => 'The :attribute must be a valid image.',
// Custom validation lines
'custom' => [

View File

@ -39,6 +39,28 @@ class ImageTest extends TestCase
]);
}
public function test_php_files_cannot_be_uploaded()
{
$page = Page::first();
$admin = $this->getAdmin();
$this->actingAs($admin);
$fileName = 'bad.php';
$relPath = $this->getTestImagePath('gallery', $fileName);
$this->deleteImage($relPath);
$file = $this->getTestImage($fileName);
$upload = $this->withHeader('Content-Type', 'image/jpeg')->call('POST', '/images/gallery/upload', ['uploaded_to' => $page->id], [], ['file' => $file], []);
$upload->assertStatus(302);
$this->assertFalse(file_exists(public_path($relPath)), 'Uploaded php file was uploaded but should have been stopped');
$this->assertDatabaseMissing('images', [
'type' => 'gallery',
'name' => $fileName
]);
}
public function test_secure_images_uploads_to_correct_place()
{
config()->set('filesystems.default', 'local_secure');

View File

@ -1,6 +1,8 @@
<?php namespace Tests\Uploads;
use Illuminate\Http\UploadedFile;
trait UsesImages
{
/**
@ -15,11 +17,11 @@ trait UsesImages
/**
* Get a test image that can be uploaded
* @param $fileName
* @return \Illuminate\Http\UploadedFile
* @return UploadedFile
*/
protected function getTestImage($fileName)
{
return new \Illuminate\Http\UploadedFile($this->getTestImageFilePath(), $fileName, 'image/png', 5238);
return new UploadedFile($this->getTestImageFilePath(), $fileName, 'image/png', 5238, null, true);
}
/**
@ -46,12 +48,14 @@ trait UsesImages
* Uploads an image with the given name.
* @param $name
* @param int $uploadedTo
* @param string $contentType
* @return \Illuminate\Foundation\Testing\TestResponse
*/
protected function uploadImage($name, $uploadedTo = 0)
protected function uploadImage($name, $uploadedTo = 0, $contentType = 'image/png')
{
$file = $this->getTestImage($name);
return $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []);
return $this->withHeader('Content-Type', $contentType)
->call('POST', '/images/gallery/upload', ['uploaded_to' => $uploadedTo], [], ['file' => $file], []);
}
/**

BIN
tests/test-data/bad.php Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 560 B