diff --git a/app/Config/app.php b/app/Config/app.php index dcd3ffc31..fc913eb8f 100644 --- a/app/Config/app.php +++ b/app/Config/app.php @@ -141,7 +141,6 @@ return [ // Third party service providers Barryvdh\DomPDF\ServiceProvider::class, Barryvdh\Snappy\ServiceProvider::class, - Intervention\Image\ImageServiceProvider::class, SocialiteProviders\Manager\ServiceProvider::class, // BookStack custom service providers @@ -161,9 +160,6 @@ return [ // Laravel Packages 'Socialite' => Laravel\Socialite\Facades\Socialite::class, - // Third Party - 'ImageTool' => Intervention\Image\Facades\Image::class, - // Custom BookStack 'Activity' => BookStack\Facades\Activity::class, 'Theme' => BookStack\Facades\Theme::class, diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index 61a1db63e..dbd4a47d2 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -211,13 +211,13 @@ class PageRepo $inputEmpty = empty($input['markdown']) && empty($input['html']); if ($haveInput && $inputEmpty) { - $pageContent->setNewHTML(''); + $pageContent->setNewHTML('', user()); } elseif (!empty($input['markdown']) && is_string($input['markdown'])) { $newEditor = 'markdown'; - $pageContent->setNewMarkdown($input['markdown']); + $pageContent->setNewMarkdown($input['markdown'], user()); } elseif (isset($input['html'])) { $newEditor = 'wysiwyg'; - $pageContent->setNewHTML($input['html']); + $pageContent->setNewHTML($input['html'], user()); } if ($newEditor !== $currentEditor && userCan('editor-change')) { @@ -284,9 +284,9 @@ class PageRepo $content = new PageContent($page); if (!empty($revision->markdown)) { - $content->setNewMarkdown($revision->markdown); + $content->setNewMarkdown($revision->markdown, user()); } else { - $content->setNewHTML($revision->html); + $content->setNewHTML($revision->html, user()); } $page->updated_by = user()->id; diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 949816eff..1bde7b6ce 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -9,7 +9,9 @@ use BookStack\Facades\Theme; use BookStack\Theming\ThemeEvents; use BookStack\Uploads\ImageRepo; use BookStack\Uploads\ImageService; +use BookStack\Users\Models\User; use BookStack\Util\HtmlContentFilter; +use BookStack\Util\WebSafeMimeSniffer; use DOMDocument; use DOMElement; use DOMNode; @@ -27,9 +29,9 @@ class PageContent /** * Update the content of the page with new provided HTML. */ - public function setNewHTML(string $html): void + public function setNewHTML(string $html, User $updater): void { - $html = $this->extractBase64ImagesFromHtml($html); + $html = $this->extractBase64ImagesFromHtml($html, $updater); $this->page->html = $this->formatHtml($html); $this->page->text = $this->toPlainText(); $this->page->markdown = ''; @@ -38,9 +40,9 @@ class PageContent /** * Update the content of the page with new provided Markdown content. */ - public function setNewMarkdown(string $markdown): void + public function setNewMarkdown(string $markdown, User $updater): void { - $markdown = $this->extractBase64ImagesFromMarkdown($markdown); + $markdown = $this->extractBase64ImagesFromMarkdown($markdown, $updater); $this->page->markdown = $markdown; $html = (new MarkdownToHtml($markdown))->convert(); $this->page->html = $this->formatHtml($html); @@ -50,7 +52,7 @@ class PageContent /** * Convert all base64 image data to saved images. */ - protected function extractBase64ImagesFromHtml(string $htmlText): string + protected function extractBase64ImagesFromHtml(string $htmlText, User $updater): string { if (empty($htmlText) || !str_contains($htmlText, 'data:image')) { return $htmlText; @@ -66,7 +68,7 @@ class PageContent $imageNodes = $xPath->query('//img[contains(@src, \'data:image\')]'); foreach ($imageNodes as $imageNode) { $imageSrc = $imageNode->getAttribute('src'); - $newUrl = $this->base64ImageUriToUploadedImageUrl($imageSrc); + $newUrl = $this->base64ImageUriToUploadedImageUrl($imageSrc, $updater); $imageNode->setAttribute('src', $newUrl); } @@ -86,7 +88,7 @@ class PageContent * Attempting to capture the whole data uri using regex can cause PHP * PCRE limits to be hit with larger, multi-MB, files. */ - protected function extractBase64ImagesFromMarkdown(string $markdown): string + protected function extractBase64ImagesFromMarkdown(string $markdown, User $updater): string { $matches = []; $contentLength = strlen($markdown); @@ -104,7 +106,7 @@ class PageContent $dataUri .= $char; } - $newUrl = $this->base64ImageUriToUploadedImageUrl($dataUri); + $newUrl = $this->base64ImageUriToUploadedImageUrl($dataUri, $updater); $replacements[] = [$dataUri, $newUrl]; } @@ -119,16 +121,28 @@ class PageContent * Parse the given base64 image URI and return the URL to the created image instance. * Returns an empty string if the parsed URI is invalid or causes an error upon upload. */ - protected function base64ImageUriToUploadedImageUrl(string $uri): string + protected function base64ImageUriToUploadedImageUrl(string $uri, User $updater): string { $imageRepo = app()->make(ImageRepo::class); $imageInfo = $this->parseBase64ImageUri($uri); + // Validate user has permission to create images + if (!$updater->can('image-create-all')) { + return ''; + } + // Validate extension and content if (empty($imageInfo['data']) || !ImageService::isExtensionSupported($imageInfo['extension'])) { return ''; } + // Validate content looks like an image via sniffing mime type + $mimeSniffer = new WebSafeMimeSniffer(); + $mime = $mimeSniffer->sniff($imageInfo['data']); + if (!str_starts_with($mime, 'image/')) { + return ''; + } + // Validate that the content is not over our upload limit $uploadLimitBytes = (config('app.upload_limit') * 1000000); if (strlen($imageInfo['data']) > $uploadLimitBytes) { diff --git a/app/Uploads/FaviconHandler.php b/app/Uploads/FaviconHandler.php index c637356e0..d5044943c 100644 --- a/app/Uploads/FaviconHandler.php +++ b/app/Uploads/FaviconHandler.php @@ -3,14 +3,13 @@ namespace BookStack\Uploads; use Illuminate\Http\UploadedFile; -use Intervention\Image\ImageManager; class FaviconHandler { protected string $path; public function __construct( - protected ImageManager $imageTool + protected ImageResizer $imageResizer, ) { $this->path = public_path('favicon.ico'); } @@ -25,10 +24,8 @@ class FaviconHandler } $imageData = file_get_contents($file->getRealPath()); - $image = $this->imageTool->make($imageData); - $image->resize(32, 32); - $bmpData = $image->encode('png'); - $icoData = $this->pngToIco($bmpData, 32, 32); + $pngData = $this->imageResizer->resizeImageData($imageData, 32, 32, false, 'png'); + $icoData = $this->pngToIco($pngData, 32, 32); file_put_contents($this->path, $icoData); } @@ -81,7 +78,7 @@ class FaviconHandler * Built following the file format info from Wikipedia: * https://en.wikipedia.org/wiki/ICO_(file_format) */ - protected function pngToIco(string $bmpData, int $width, int $height): string + protected function pngToIco(string $pngData, int $width, int $height): string { // ICO header $header = pack('v', 0x00); // Reserved. Must always be 0 @@ -100,11 +97,11 @@ class FaviconHandler // via intervention from png typically provides this as 24. $entry .= pack('v', 0x00); // Size of the image data in bytes - $entry .= pack('V', strlen($bmpData)); + $entry .= pack('V', strlen($pngData)); // Offset of the bmp data from file start $entry .= pack('V', strlen($header) + strlen($entry) + 4); // Join & return the combined parts of the ICO image data - return $header . $entry . $bmpData; + return $header . $entry . $pngData; } } diff --git a/app/Uploads/ImageResizer.php b/app/Uploads/ImageResizer.php index 0d090a94b..4dc1b0b99 100644 --- a/app/Uploads/ImageResizer.php +++ b/app/Uploads/ImageResizer.php @@ -6,15 +6,14 @@ use BookStack\Exceptions\ImageUploadException; use Exception; use GuzzleHttp\Psr7\Utils; use Illuminate\Support\Facades\Cache; +use Intervention\Image\Gd\Driver; use Intervention\Image\Image as InterventionImage; -use Intervention\Image\ImageManager; class ImageResizer { protected const THUMBNAIL_CACHE_TIME = 604_800; // 1 week public function __construct( - protected ImageManager $intervention, protected ImageStorage $storage, ) { } @@ -105,13 +104,19 @@ class ImageResizer /** * Resize the image of given data to the specified size, and return the new image data. + * Format will remain the same as the input format, unless specified. * * @throws ImageUploadException */ - public function resizeImageData(string $imageData, ?int $width, ?int $height, bool $keepRatio): string - { + public function resizeImageData( + string $imageData, + ?int $width, + ?int $height, + bool $keepRatio, + ?string $format = null, + ): string { try { - $thumb = $this->intervention->make($imageData); + $thumb = $this->interventionFromImageData($imageData); } catch (Exception $e) { throw new ImageUploadException(trans('errors.cannot_create_thumbs')); } @@ -127,7 +132,7 @@ class ImageResizer $thumb->fit($width, $height); } - $thumbData = (string) $thumb->encode(); + $thumbData = (string) $thumb->encode($format); // Use original image data if we're keeping the ratio // and the resizing does not save any space. @@ -138,6 +143,17 @@ class ImageResizer return $thumbData; } + /** + * Create an intervention image instance from the given image data. + * Performs some manual library usage to ensure image is specifically loaded + * from given binary data instead of data being misinterpreted. + */ + protected function interventionFromImageData(string $imageData): InterventionImage + { + $driver = new Driver(); + return $driver->decoder->initFromBinary($imageData); + } + /** * Orientate the given intervention image based upon the given original image data. * Intervention does have an `orientate` method but the exif data it needs is lost before it diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index 5bd308ae8..3797e7cb0 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -160,10 +160,6 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function can(string $permissionName): bool { - if ($this->email === 'guest') { - return false; - } - return $this->permissions()->contains($permissionName); } diff --git a/resources/js/services/util.js b/resources/js/services/util.js index e6d93d61b..59affff88 100644 --- a/resources/js/services/util.js +++ b/resources/js/services/util.js @@ -1,5 +1,3 @@ -import {elem} from './dom'; - /** * Returns a function, that, as long as it continues to be invoked, will not * be triggered. The function will be called after it stops being called for diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index d8845fe12..78b739512 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -8,7 +8,7 @@ use Tests\TestCase; class PageContentTest extends TestCase { - protected $base64Jpeg = '/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k='; + protected string $base64Jpeg = '/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k='; public function test_page_includes() { @@ -649,6 +649,35 @@ class PageContentTest extends TestCase } } + public function test_base64_images_within_html_blanked_if_no_image_create_permission() + { + $editor = $this->users->editor(); + $page = $this->entities->page(); + $this->permissions->removeUserRolePermissions($editor, ['image-create-all']); + + $this->actingAs($editor)->put($page->getUrl(), [ + 'name' => $page->name, + 'html' => '
test
test%A
test
test%A