Compare commits

...

6 Commits

Author SHA1 Message Date
Dan Brown
c3e74219c4 Updated version and assets for release v0.25.4 2019-03-21 19:46:19 +00:00
Dan Brown
13c9d7bc2d Merge branch 'master' into release 2019-03-21 19:43:48 +00:00
Dan Brown
f5fe524e6c Added extension whitelist for image uploads
- A continuation of the security issues addressed in v0.25.3
2019-03-21 19:43:15 +00:00
Dan Brown
119b539586 Updated version and assets for release v0.25.3 2019-03-21 00:03:26 +00:00
Dan Brown
29a5c180f0 Merge branch 'master' into release 2019-03-21 00:02:33 +00:00
Dan Brown
37b91b6b0e Hardened image file validation by removing custom validation
- Added test to check PHP files cannot be uploaded as an image.
2019-03-20 23:59:55 +00:00
8 changed files with 66 additions and 17 deletions

View File

@@ -119,7 +119,7 @@ class ImageController extends Controller
{
$this->checkPermission('image-create-all');
$this->validate($request, [
'file' => 'is_image'
'file' => 'image_extension|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

@@ -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;
@@ -22,11 +23,12 @@ 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);
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 "<?php echo icon($expression); ?>";

View File

@@ -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.',
@@ -69,7 +70,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,13 +39,57 @@ 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_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);
@@ -61,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'];
@@ -84,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);

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

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

Binary file not shown.

After

Width:  |  Height:  |  Size: 560 B

View File

@@ -1 +1 @@
v0.25.2
v0.25.4