diff --git a/app/Http/Controllers/ImageController.php b/app/Http/Controllers/ImageController.php index a6c92bccc..fdcb3aba4 100644 --- a/app/Http/Controllers/ImageController.php +++ b/app/Http/Controllers/ImageController.php @@ -119,7 +119,7 @@ class ImageController extends Controller { $this->checkPermission('image-create-all'); $this->validate($request, [ - 'file' => 'mimes:jpeg,png,gif,bmp,webp,tiff' + 'file' => 'image_extension|mimes:jpeg,png,gif,bmp,webp,tiff' ]); if (!$this->imageRepo->isValidType($type)) { diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 77154baac..1d9a8736e 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -8,6 +8,7 @@ use BookStack\Entities\Page; use BookStack\Settings\Setting; use BookStack\Settings\SettingService; use Illuminate\Database\Eloquent\Relations\Relation; +use Illuminate\Http\UploadedFile; use Illuminate\Support\ServiceProvider; use Schema; use Validator; @@ -21,6 +22,13 @@ class AppServiceProvider extends ServiceProvider */ public function boot() { + // Custom validation methods + Validator::extend('image_extension', function ($attribute, $value, $parameters, $validator) { + $validImageExtensions = ['png', 'jpg', 'jpeg', 'bmp', 'gif', 'tiff', 'webp']; + return in_array(strtolower($value->getClientOriginalExtension()), $validImageExtensions); + }); + + // Custom blade view directives Blade::directive('icon', function ($expression) { return ""; diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index 0b6a1c170..3de4c54b0 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -33,6 +33,7 @@ return [ 'filled' => 'The :attribute field is required.', 'exists' => 'The selected :attribute is invalid.', 'image' => 'The :attribute must be an image.', + 'image_extension' => 'The :attribute must have a valid & supported image extension.', 'in' => 'The selected :attribute is invalid.', 'integer' => 'The :attribute must be an integer.', 'ip' => 'The :attribute must be a valid IP address.', diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php index e26456824..4091bec57 100644 --- a/tests/Uploads/ImageTest.php +++ b/tests/Uploads/ImageTest.php @@ -61,13 +61,35 @@ class ImageTest extends TestCase ]); } + public function test_php_like_files_cannot_be_uploaded() + { + $page = Page::first(); + $admin = $this->getAdmin(); + $this->actingAs($admin); + + $fileName = 'bad.phtml'; + $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'); $this->asEditor(); - $galleryFile = $this->getTestImage('my-secure-test-upload'); + $galleryFile = $this->getTestImage('my-secure-test-upload.png'); $page = Page::first(); - $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m-M') . '/my-secure-test-upload'); + $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m-M') . '/my-secure-test-upload.png'); $upload = $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []); $upload->assertStatus(200); @@ -83,9 +105,9 @@ class ImageTest extends TestCase { config()->set('filesystems.default', 'local_secure'); $this->asEditor(); - $galleryFile = $this->getTestImage('my-secure-test-upload'); + $galleryFile = $this->getTestImage('my-secure-test-upload.png'); $page = Page::first(); - $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m-M') . '/my-secure-test-upload'); + $expectedPath = storage_path('uploads/images/gallery/' . Date('Y-m-M') . '/my-secure-test-upload.png'); $upload = $this->call('POST', '/images/gallery/upload', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []); $imageUrl = json_decode($upload->getContent(), true)['url']; @@ -106,9 +128,9 @@ class ImageTest extends TestCase { config()->set('filesystems.default', 'local_secure'); $this->asEditor(); - $galleryFile = $this->getTestImage('my-system-test-upload'); + $galleryFile = $this->getTestImage('my-system-test-upload.png'); $page = Page::first(); - $expectedPath = public_path('uploads/images/system/' . Date('Y-m-M') . '/my-system-test-upload'); + $expectedPath = public_path('uploads/images/system/' . Date('Y-m-M') . '/my-system-test-upload.png'); $upload = $this->call('POST', '/images/system/upload', ['uploaded_to' => $page->id], [], ['file' => $galleryFile], []); $upload->assertStatus(200); diff --git a/tests/test-data/bad.phtml b/tests/test-data/bad.phtml new file mode 100644 index 000000000..3b7c0f36c Binary files /dev/null and b/tests/test-data/bad.phtml differ