From 5fd10e695a4dd9ac34b35da49ee52bb1b429b045 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 29 Oct 2021 21:37:10 +0100 Subject: [PATCH 01/10] Added sponsors to readme, updated license file --- LICENSE | 2 +- readme.md | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/LICENSE b/LICENSE index 61aeaad8c..0ec2e91ab 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2020 Dan Brown and the BookStack Project contributors +Copyright (c) 2015-present, Dan Brown and the BookStack Project contributors https://github.com/BookStackApp/BookStack/graphs/contributors Permission is hereby granted, free of charge, to any person obtaining a copy diff --git a/readme.md b/readme.md index 17ac9641b..eefbe98b6 100644 --- a/readme.md +++ b/readme.md @@ -27,6 +27,25 @@ BookStack is not designed as an extensible platform to be used for purposes that In regard to development philosophy, BookStack has a relaxed, open & positive approach. At the end of the day this is free software developed and maintained by people donating their own free time. +## 🌟 Project Sponsors + +Shown below are our bronze, silver and gold project sponsors. +Big thanks to these companies for supporting the project. +Note: Listed services are not tested, vetted nor supported by the official BookStack project in any manner. +[View all sponsors](https://github.com/sponsors/ssddanbrown). + +#### Bronze Sponsors + + + + + +
+ Diagrams.net logo + + Stellar Hosted Logo +
+ ## 🛣️ Road Map Below is a high-level road map view for BookStack to provide a sense of direction of where the project is going. This can change at any point and does not reflect many features and improvements that will also be included as part of the journey along this road map. For more granular detail of what will be included in upcoming releases you can review the project milestones as defined in the "Release Process" section below. From 85dc8d97918b75bf3bf465876df825018e2185d4 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 30 Oct 2021 11:51:49 +0100 Subject: [PATCH 02/10] Updated sponsor link --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index eefbe98b6..2db6cee20 100644 --- a/readme.md +++ b/readme.md @@ -41,7 +41,7 @@ Note: Listed services are not tested, vetted nor supported by the official BookS Diagrams.net logo - + Stellar Hosted Logo From 5c834f24a6e2b933236cbabc4484f00c686dc05c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 31 Oct 2021 13:08:01 +0000 Subject: [PATCH 03/10] Updated AzureAD provider to use microsoft graph Since AzureAD graph is going away. Tested using old AzureAD graph usage for backwards-compatbility, did not seem to break things. Could not test with conditional access though due to azure never enforcing it no matter what I attempted. Fpr #3028 --- app/Auth/Access/SocialAuthService.php | 3 - composer.json | 2 +- composer.lock | 138 ++++++++++++++------------ 3 files changed, 75 insertions(+), 68 deletions(-) diff --git a/app/Auth/Access/SocialAuthService.php b/app/Auth/Access/SocialAuthService.php index d165e76b1..23e95970c 100644 --- a/app/Auth/Access/SocialAuthService.php +++ b/app/Auth/Access/SocialAuthService.php @@ -281,9 +281,6 @@ class SocialAuthService if ($driverName === 'google' && config('services.google.select_account')) { $driver->with(['prompt' => 'select_account']); } - if ($driverName === 'azure') { - $driver->with(['resource' => 'https://graph.windows.net']); - } if (isset($this->configureForRedirectCallbacks[$driverName])) { $this->configureForRedirectCallbacks[$driverName]($driver); diff --git a/composer.json b/composer.json index fa2c0c2b5..addde9b7e 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "predis/predis": "^1.1.6", "socialiteproviders/discord": "^4.1", "socialiteproviders/gitlab": "^4.1", - "socialiteproviders/microsoft-azure": "^4.1", + "socialiteproviders/microsoft-azure": "^5.0.1", "socialiteproviders/okta": "^4.1", "socialiteproviders/slack": "^4.1", "socialiteproviders/twitch": "^5.3", diff --git a/composer.lock b/composer.lock index 318544c5a..9364f7b7c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "fc6d8f731e3975127a9101802cc4bb3a", + "content-hash": "4780c46ffc4d1a09af810f62ca59e4a7", "packages": [ { "name": "aws/aws-crt-php", @@ -58,16 +58,16 @@ }, { "name": "aws/aws-sdk-php", - "version": "3.199.3", + "version": "3.199.7", "source": { "type": "git", "url": "https://github.com/aws/aws-sdk-php.git", - "reference": "132a1148ebb63d04023837bcf9a36f49b308a0bd" + "reference": "fda176884d2952cffc7e67209470bff49609339c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/aws/aws-sdk-php/zipball/132a1148ebb63d04023837bcf9a36f49b308a0bd", - "reference": "132a1148ebb63d04023837bcf9a36f49b308a0bd", + "url": "https://api.github.com/repos/aws/aws-sdk-php/zipball/fda176884d2952cffc7e67209470bff49609339c", + "reference": "fda176884d2952cffc7e67209470bff49609339c", "shasum": "" }, "require": { @@ -143,9 +143,9 @@ "support": { "forum": "https://forums.aws.amazon.com/forum.jspa?forumID=80", "issues": "https://github.com/aws/aws-sdk-php/issues", - "source": "https://github.com/aws/aws-sdk-php/tree/3.199.3" + "source": "https://github.com/aws/aws-sdk-php/tree/3.199.7" }, - "time": "2021-10-25T18:17:28+00:00" + "time": "2021-10-29T18:25:02+00:00" }, { "name": "bacon/bacon-qr-code", @@ -3297,16 +3297,16 @@ }, { "name": "phpseclib/phpseclib", - "version": "3.0.10", + "version": "3.0.11", "source": { "type": "git", "url": "https://github.com/phpseclib/phpseclib.git", - "reference": "62fcc5a94ac83b1506f52d7558d828617fac9187" + "reference": "6e794226a35159eb06f355efe59a0075a16551dd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpseclib/phpseclib/zipball/62fcc5a94ac83b1506f52d7558d828617fac9187", - "reference": "62fcc5a94ac83b1506f52d7558d828617fac9187", + "url": "https://api.github.com/repos/phpseclib/phpseclib/zipball/6e794226a35159eb06f355efe59a0075a16551dd", + "reference": "6e794226a35159eb06f355efe59a0075a16551dd", "shasum": "" }, "require": { @@ -3388,7 +3388,7 @@ ], "support": { "issues": "https://github.com/phpseclib/phpseclib/issues", - "source": "https://github.com/phpseclib/phpseclib/tree/3.0.10" + "source": "https://github.com/phpseclib/phpseclib/tree/3.0.11" }, "funding": [ { @@ -3404,7 +3404,7 @@ "type": "tidelift" } ], - "time": "2021-08-16T04:24:45+00:00" + "time": "2021-10-27T03:01:46+00:00" }, { "name": "pragmarx/google2fa", @@ -4224,16 +4224,16 @@ }, { "name": "socialiteproviders/microsoft-azure", - "version": "4.2.1", + "version": "5.0.1", "source": { "type": "git", "url": "https://github.com/SocialiteProviders/Microsoft-Azure.git", - "reference": "64779ec21db0bee3111039a67c0fa0ab550a3462" + "reference": "9b23e02ff711de42e513aa55f768a4f1c67c0e41" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/SocialiteProviders/Microsoft-Azure/zipball/64779ec21db0bee3111039a67c0fa0ab550a3462", - "reference": "64779ec21db0bee3111039a67c0fa0ab550a3462", + "url": "https://api.github.com/repos/SocialiteProviders/Microsoft-Azure/zipball/9b23e02ff711de42e513aa55f768a4f1c67c0e41", + "reference": "9b23e02ff711de42e513aa55f768a4f1c67c0e41", "shasum": "" }, "require": { @@ -4258,10 +4258,20 @@ } ], "description": "Microsoft Azure OAuth2 Provider for Laravel Socialite", + "keywords": [ + "azure", + "laravel", + "microsoft", + "oauth", + "provider", + "socialite" + ], "support": { - "source": "https://github.com/SocialiteProviders/Microsoft-Azure/tree/4.2.1" + "docs": "https://socialiteproviders.com/microsoft-azure", + "issues": "https://github.com/socialiteproviders/providers/issues", + "source": "https://github.com/socialiteproviders/providers" }, - "time": "2021-06-14T22:51:38+00:00" + "time": "2021-10-07T22:21:59+00:00" }, { "name": "socialiteproviders/okta", @@ -4515,16 +4525,16 @@ }, { "name": "symfony/console", - "version": "v4.4.30", + "version": "v4.4.33", "source": { "type": "git", "url": "https://github.com/symfony/console.git", - "reference": "a3f7189a0665ee33b50e9e228c46f50f5acbed22" + "reference": "8dbd23ef7a8884051482183ddee8d9061b5feed0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/console/zipball/a3f7189a0665ee33b50e9e228c46f50f5acbed22", - "reference": "a3f7189a0665ee33b50e9e228c46f50f5acbed22", + "url": "https://api.github.com/repos/symfony/console/zipball/8dbd23ef7a8884051482183ddee8d9061b5feed0", + "reference": "8dbd23ef7a8884051482183ddee8d9061b5feed0", "shasum": "" }, "require": { @@ -4585,7 +4595,7 @@ "description": "Eases the creation of beautiful and testable command line interfaces", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/console/tree/v4.4.30" + "source": "https://github.com/symfony/console/tree/v4.4.33" }, "funding": [ { @@ -4601,7 +4611,7 @@ "type": "tidelift" } ], - "time": "2021-08-25T19:27:26+00:00" + "time": "2021-10-25T16:36:08+00:00" }, { "name": "symfony/css-selector", @@ -5177,16 +5187,16 @@ }, { "name": "symfony/http-foundation", - "version": "v4.4.30", + "version": "v4.4.33", "source": { "type": "git", "url": "https://github.com/symfony/http-foundation.git", - "reference": "09b3202651ab23ac8dcf455284a48a3500e56731" + "reference": "b9a91102f548e0111f4996e8c622fb1d1d479850" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/http-foundation/zipball/09b3202651ab23ac8dcf455284a48a3500e56731", - "reference": "09b3202651ab23ac8dcf455284a48a3500e56731", + "url": "https://api.github.com/repos/symfony/http-foundation/zipball/b9a91102f548e0111f4996e8c622fb1d1d479850", + "reference": "b9a91102f548e0111f4996e8c622fb1d1d479850", "shasum": "" }, "require": { @@ -5225,7 +5235,7 @@ "description": "Defines an object-oriented layer for the HTTP specification", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/http-foundation/tree/v4.4.30" + "source": "https://github.com/symfony/http-foundation/tree/v4.4.33" }, "funding": [ { @@ -5241,20 +5251,20 @@ "type": "tidelift" } ], - "time": "2021-08-26T15:51:23+00:00" + "time": "2021-10-07T15:31:35+00:00" }, { "name": "symfony/http-kernel", - "version": "v4.4.32", + "version": "v4.4.33", "source": { "type": "git", "url": "https://github.com/symfony/http-kernel.git", - "reference": "f7bda3ea8f05ae90627400e58af5179b25ce0f38" + "reference": "6f1fcca1154f782796549f4f4e5090bae9525c0e" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/http-kernel/zipball/f7bda3ea8f05ae90627400e58af5179b25ce0f38", - "reference": "f7bda3ea8f05ae90627400e58af5179b25ce0f38", + "url": "https://api.github.com/repos/symfony/http-kernel/zipball/6f1fcca1154f782796549f4f4e5090bae9525c0e", + "reference": "6f1fcca1154f782796549f4f4e5090bae9525c0e", "shasum": "" }, "require": { @@ -5329,7 +5339,7 @@ "description": "Provides a structured process for converting a Request into a Response", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/http-kernel/tree/v4.4.32" + "source": "https://github.com/symfony/http-kernel/tree/v4.4.33" }, "funding": [ { @@ -5345,7 +5355,7 @@ "type": "tidelift" } ], - "time": "2021-09-28T10:20:04+00:00" + "time": "2021-10-29T08:14:01+00:00" }, { "name": "symfony/mime", @@ -6477,16 +6487,16 @@ }, { "name": "symfony/var-dumper", - "version": "v4.4.31", + "version": "v4.4.33", "source": { "type": "git", "url": "https://github.com/symfony/var-dumper.git", - "reference": "1f12cc0c2e880a5f39575c19af81438464717839" + "reference": "50286e2b7189bfb4f419c0731e86632cddf7c5ee" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/var-dumper/zipball/1f12cc0c2e880a5f39575c19af81438464717839", - "reference": "1f12cc0c2e880a5f39575c19af81438464717839", + "url": "https://api.github.com/repos/symfony/var-dumper/zipball/50286e2b7189bfb4f419c0731e86632cddf7c5ee", + "reference": "50286e2b7189bfb4f419c0731e86632cddf7c5ee", "shasum": "" }, "require": { @@ -6546,7 +6556,7 @@ "dump" ], "support": { - "source": "https://github.com/symfony/var-dumper/tree/v4.4.31" + "source": "https://github.com/symfony/var-dumper/tree/v4.4.33" }, "funding": [ { @@ -6562,7 +6572,7 @@ "type": "tidelift" } ], - "time": "2021-09-24T15:30:11+00:00" + "time": "2021-10-25T20:24:58+00:00" }, { "name": "tijsverkoyen/css-to-inline-styles", @@ -6919,16 +6929,16 @@ }, { "name": "composer/ca-bundle", - "version": "1.2.11", + "version": "1.3.1", "source": { "type": "git", "url": "https://github.com/composer/ca-bundle.git", - "reference": "0b072d51c5a9c6f3412f7ea3ab043d6603cb2582" + "reference": "4c679186f2aca4ab6a0f1b0b9cf9252decb44d0b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/composer/ca-bundle/zipball/0b072d51c5a9c6f3412f7ea3ab043d6603cb2582", - "reference": "0b072d51c5a9c6f3412f7ea3ab043d6603cb2582", + "url": "https://api.github.com/repos/composer/ca-bundle/zipball/4c679186f2aca4ab6a0f1b0b9cf9252decb44d0b", + "reference": "4c679186f2aca4ab6a0f1b0b9cf9252decb44d0b", "shasum": "" }, "require": { @@ -6975,7 +6985,7 @@ "support": { "irc": "irc://irc.freenode.org/composer", "issues": "https://github.com/composer/ca-bundle/issues", - "source": "https://github.com/composer/ca-bundle/tree/1.2.11" + "source": "https://github.com/composer/ca-bundle/tree/1.3.1" }, "funding": [ { @@ -6991,20 +7001,20 @@ "type": "tidelift" } ], - "time": "2021-09-25T20:32:43+00:00" + "time": "2021-10-28T20:44:15+00:00" }, { "name": "composer/composer", - "version": "2.1.9", + "version": "2.1.10", "source": { "type": "git", "url": "https://github.com/composer/composer.git", - "reference": "e558c88f28d102d497adec4852802c0dc14c7077" + "reference": "ea5f64d1a15c66942979b804c9fb3686be852ca0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/composer/composer/zipball/e558c88f28d102d497adec4852802c0dc14c7077", - "reference": "e558c88f28d102d497adec4852802c0dc14c7077", + "url": "https://api.github.com/repos/composer/composer/zipball/ea5f64d1a15c66942979b804c9fb3686be852ca0", + "reference": "ea5f64d1a15c66942979b804c9fb3686be852ca0", "shasum": "" }, "require": { @@ -7015,7 +7025,7 @@ "composer/xdebug-handler": "^2.0", "justinrainbow/json-schema": "^5.2.11", "php": "^5.3.2 || ^7.0 || ^8.0", - "psr/log": "^1.0", + "psr/log": "^1.0 || ^2.0", "react/promise": "^1.2 || ^2.7", "seld/jsonlint": "^1.4", "seld/phar-utils": "^1.0", @@ -7039,7 +7049,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "2.1-dev" + "dev-main": "2.1-dev" } }, "autoload": { @@ -7073,7 +7083,7 @@ "support": { "irc": "ircs://irc.libera.chat:6697/composer", "issues": "https://github.com/composer/composer/issues", - "source": "https://github.com/composer/composer/tree/2.1.9" + "source": "https://github.com/composer/composer/tree/2.1.10" }, "funding": [ { @@ -7089,7 +7099,7 @@ "type": "tidelift" } ], - "time": "2021-10-05T07:47:38+00:00" + "time": "2021-10-29T20:34:57+00:00" }, { "name": "composer/metadata-minifier", @@ -8230,23 +8240,23 @@ }, { "name": "phpunit/php-code-coverage", - "version": "9.2.7", + "version": "9.2.8", "source": { "type": "git", "url": "https://github.com/sebastianbergmann/php-code-coverage.git", - "reference": "d4c798ed8d51506800b441f7a13ecb0f76f12218" + "reference": "cf04e88a2e3c56fc1a65488afd493325b4c1bc3e" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/d4c798ed8d51506800b441f7a13ecb0f76f12218", - "reference": "d4c798ed8d51506800b441f7a13ecb0f76f12218", + "url": "https://api.github.com/repos/sebastianbergmann/php-code-coverage/zipball/cf04e88a2e3c56fc1a65488afd493325b4c1bc3e", + "reference": "cf04e88a2e3c56fc1a65488afd493325b4c1bc3e", "shasum": "" }, "require": { "ext-dom": "*", "ext-libxml": "*", "ext-xmlwriter": "*", - "nikic/php-parser": "^4.12.0", + "nikic/php-parser": "^4.13.0", "php": ">=7.3", "phpunit/php-file-iterator": "^3.0.3", "phpunit/php-text-template": "^2.0.2", @@ -8295,7 +8305,7 @@ ], "support": { "issues": "https://github.com/sebastianbergmann/php-code-coverage/issues", - "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.7" + "source": "https://github.com/sebastianbergmann/php-code-coverage/tree/9.2.8" }, "funding": [ { @@ -8303,7 +8313,7 @@ "type": "github" } ], - "time": "2021-09-17T05:39:03+00:00" + "time": "2021-10-30T08:01:38+00:00" }, { "name": "phpunit/php-file-iterator", From ae155d67454d6b9f6c93b2bb457aaa4b2eb1a9ed Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 31 Oct 2021 17:58:56 +0000 Subject: [PATCH 04/10] Added safe mime sniffing to prevent serving HTML (Amoung other content types) For #3027 --- app/Http/Controllers/Controller.php | 15 ++++--- app/Util/WebSafeMimeSniffer.php | 65 +++++++++++++++++++++++++++++ tests/Uploads/AttachmentTest.php | 32 ++++++++++++++ 3 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 app/Util/WebSafeMimeSniffer.php diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 283a01cfb..3bccdcda4 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -5,6 +5,7 @@ namespace BookStack\Http\Controllers; use BookStack\Facades\Activity; use BookStack\Interfaces\Loggable; use BookStack\Model; +use BookStack\Util\WebSafeMimeSniffer; use finfo; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Foundation\Validation\ValidatesRequests; @@ -117,8 +118,9 @@ abstract class Controller extends BaseController protected function downloadResponse(string $content, string $fileName): Response { return response()->make($content, 200, [ - 'Content-Type' => 'application/octet-stream', - 'Content-Disposition' => 'attachment; filename="' . $fileName . '"', + 'Content-Type' => 'application/octet-stream', + 'Content-Disposition' => 'attachment; filename="' . $fileName . '"', + 'X-Content-Type-Options' => 'nosniff', ]); } @@ -128,12 +130,13 @@ abstract class Controller extends BaseController */ protected function inlineDownloadResponse(string $content, string $fileName): Response { - $finfo = new finfo(FILEINFO_MIME_TYPE); - $mime = $finfo->buffer($content) ?: 'application/octet-stream'; + + $mime = (new WebSafeMimeSniffer)->sniff($content); return response()->make($content, 200, [ - 'Content-Type' => $mime, - 'Content-Disposition' => 'inline; filename="' . $fileName . '"', + 'Content-Type' => $mime, + 'Content-Disposition' => 'inline; filename="' . $fileName . '"', + 'X-Content-Type-Options' => 'nosniff', ]); } diff --git a/app/Util/WebSafeMimeSniffer.php b/app/Util/WebSafeMimeSniffer.php new file mode 100644 index 000000000..a896bd9e5 --- /dev/null +++ b/app/Util/WebSafeMimeSniffer.php @@ -0,0 +1,65 @@ +buffer($content) ?: 'application/octet-stream'; + + if (in_array($mime, $this->safeMimes)) { + return $mime; + } + + [$category] = explode('/', $mime, 2); + if ($category === 'text') { + return 'text/plain'; + } + + return 'application/octet-stream'; + } + +} \ No newline at end of file diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 60fd370b6..26f092bcc 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -9,6 +9,7 @@ use BookStack\Uploads\Attachment; use BookStack\Uploads\AttachmentService; use Illuminate\Http\UploadedFile; use Tests\TestCase; +use Tests\TestResponse; class AttachmentTest extends TestCase { @@ -44,6 +45,20 @@ class AttachmentTest extends TestCase return Attachment::query()->latest()->first(); } + /** + * Create a new upload attachment from the given data. + */ + protected function createUploadAttachment(Page $page, string $filename, string $content, string $mimeType): Attachment + { + $file = tmpfile(); + $filePath = stream_get_meta_data($file)['uri']; + file_put_contents($filePath, $content); + $upload = new UploadedFile($filePath, $filename, $mimeType, null, true); + + $this->call('POST', '/attachments/upload', ['uploaded_to' => $page->id], [], ['file' => $upload], []); + return $page->attachments()->latest()->firstOrFail(); + } + /** * Delete all uploaded files. * To assist with cleanup. @@ -305,7 +320,24 @@ class AttachmentTest extends TestCase // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=UTF-8'); $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="upload_test_file.txt"'); + $attachmentGet->assertHeader('X-Content-Type-Options', 'nosniff'); $this->deleteUploads(); } + + public function test_html_file_access_with_open_forces_plain_content_type() + { + $page = Page::query()->first(); + $this->asAdmin(); + + $attachment = $this->createUploadAttachment($page, 'test_file.html', '

testing

', 'text/html'); + + $attachmentGet = $this->get($attachment->getUrl(true)); + // http-foundation/Response does some 'fixing' of responses to add charsets to text responses. + $attachmentGet->assertHeader('Content-Type', 'text/plain; charset=UTF-8'); + $attachmentGet->assertHeader('Content-Disposition', 'inline; filename="test_file.html"'); + + $this->deleteUploads(); + } + } From 43830a372fc51a8793199d04a34c3f4ebdfccc7b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 31 Oct 2021 23:53:17 +0000 Subject: [PATCH 05/10] Updated showImage file serving to not be traversable For #3030 --- .../Controllers/Images/ImageController.php | 13 +++-- app/Uploads/AttachmentService.php | 8 ++-- app/Uploads/ImageService.php | 47 ++++++++++++++++--- tests/Uploads/ImageTest.php | 30 ++++++++++++ 4 files changed, 84 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/Images/ImageController.php b/app/Http/Controllers/Images/ImageController.php index 4070a0e2f..66ccadc5e 100644 --- a/app/Http/Controllers/Images/ImageController.php +++ b/app/Http/Controllers/Images/ImageController.php @@ -7,25 +7,31 @@ use BookStack\Exceptions\NotFoundException; use BookStack\Http\Controllers\Controller; use BookStack\Uploads\Image; use BookStack\Uploads\ImageRepo; +use BookStack\Uploads\ImageService; use Exception; use Illuminate\Filesystem\Filesystem as File; +use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Storage; use Illuminate\Validation\ValidationException; +use League\Flysystem\Util; class ImageController extends Controller { protected $image; protected $file; protected $imageRepo; + protected $imageService; /** * ImageController constructor. */ - public function __construct(Image $image, File $file, ImageRepo $imageRepo) + public function __construct(Image $image, File $file, ImageRepo $imageRepo, ImageService $imageService) { $this->image = $image; $this->file = $file; $this->imageRepo = $imageRepo; + $this->imageService = $imageService; } /** @@ -35,14 +41,13 @@ class ImageController extends Controller */ public function showImage(string $path) { - $path = storage_path('uploads/images/' . $path); - if (!file_exists($path)) { + if (!$this->imageService->pathExistsInLocalSecure($path)) { throw (new NotFoundException(trans('errors.image_not_found'))) ->setSubtitle(trans('errors.image_not_found_subtitle')) ->setDetails(trans('errors.image_not_found_details')); } - return response()->file($path); + return $this->imageService->streamImageFromStorageResponse('gallery', $path); } /** diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php index f7a0918c6..c9cd99b38 100644 --- a/app/Uploads/AttachmentService.php +++ b/app/Uploads/AttachmentService.php @@ -27,7 +27,7 @@ class AttachmentService /** * Get the storage that will be used for storing files. */ - protected function getStorage(): FileSystemInstance + protected function getStorageDisk(): FileSystemInstance { return $this->fileSystem->disk($this->getStorageDiskName()); } @@ -70,7 +70,7 @@ class AttachmentService */ public function getAttachmentFromStorage(Attachment $attachment): string { - return $this->getStorage()->get($this->adjustPathForStorageDisk($attachment->path)); + return $this->getStorageDisk()->get($this->adjustPathForStorageDisk($attachment->path)); } /** @@ -195,7 +195,7 @@ class AttachmentService */ protected function deleteFileInStorage(Attachment $attachment) { - $storage = $this->getStorage(); + $storage = $this->getStorageDisk(); $dirPath = $this->adjustPathForStorageDisk(dirname($attachment->path)); $storage->delete($this->adjustPathForStorageDisk($attachment->path)); @@ -213,7 +213,7 @@ class AttachmentService { $attachmentData = file_get_contents($uploadedFile->getRealPath()); - $storage = $this->getStorage(); + $storage = $this->getStorageDisk(); $basePath = 'uploads/files/' . date('Y-m-M') . '/'; $uploadFileName = Str::random(16) . '.' . $uploadedFile->getClientOriginalExtension(); diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index d6c74c751..dc18783c5 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -16,6 +16,7 @@ use Intervention\Image\Exception\NotSupportedException; use Intervention\Image\ImageManager; use League\Flysystem\Util; use Symfony\Component\HttpFoundation\File\UploadedFile; +use Symfony\Component\HttpFoundation\StreamedResponse; class ImageService { @@ -39,11 +40,20 @@ class ImageService /** * Get the storage that will be used for storing images. */ - protected function getStorage(string $imageType = ''): FileSystemInstance + protected function getStorageDisk(string $imageType = ''): FileSystemInstance { return $this->fileSystem->disk($this->getStorageDiskName($imageType)); } + /** + * Check if local secure image storage (Fetched behind authentication) + * is currently active in the instance. + */ + protected function usingSecureImages(): bool + { + return $this->getStorageDiskName('gallery') === 'local_secure_images'; + } + /** * Change the originally provided path to fit any disk-specific requirements. * This also ensures the path is kept to the expected root folders. @@ -126,7 +136,7 @@ class ImageService */ public function saveNew(string $imageName, string $imageData, string $type, int $uploadedTo = 0): Image { - $storage = $this->getStorage($type); + $storage = $this->getStorageDisk($type); $secureUploads = setting('app-secure-images'); $fileName = $this->cleanImageFileName($imageName); @@ -243,7 +253,7 @@ class ImageService return $this->getPublicUrl($thumbFilePath); } - $storage = $this->getStorage($image->type); + $storage = $this->getStorageDisk($image->type); if ($storage->exists($this->adjustPathForStorageDisk($thumbFilePath, $image->type))) { return $this->getPublicUrl($thumbFilePath); } @@ -307,7 +317,7 @@ class ImageService */ public function getImageData(Image $image): string { - $storage = $this->getStorage(); + $storage = $this->getStorageDisk(); return $storage->get($this->adjustPathForStorageDisk($image->path, $image->type)); } @@ -330,7 +340,7 @@ class ImageService protected function destroyImagesFromPath(string $path, string $imageType): bool { $path = $this->adjustPathForStorageDisk($path, $imageType); - $storage = $this->getStorage($imageType); + $storage = $this->getStorageDisk($imageType); $imageFolder = dirname($path); $imageFileName = basename($path); @@ -417,7 +427,7 @@ class ImageService } $storagePath = $this->adjustPathForStorageDisk($storagePath); - $storage = $this->getStorage(); + $storage = $this->getStorageDisk(); $imageData = null; if ($storage->exists($storagePath)) { $imageData = $storage->get($storagePath); @@ -435,6 +445,31 @@ class ImageService return 'data:image/' . $extension . ';base64,' . base64_encode($imageData); } + /** + * Check if the given path exists in the local secure image system. + * Returns false if local_secure is not in use. + */ + public function pathExistsInLocalSecure(string $imagePath): bool + { + $disk = $this->getStorageDisk('gallery'); + + // Check local_secure is active + return $this->usingSecureImages() + // Check the image file exists + && $disk->exists($imagePath) + // Check the file is likely an image file + && strpos($disk->getMimetype($imagePath), 'image/') === 0; + } + + /** + * For the given path, if existing, provide a response that will stream the image contents. + */ + public function streamImageFromStorageResponse(string $imageType, string $path): StreamedResponse + { + $disk = $this->getStorageDisk($imageType); + return $disk->response($path); + } + /** * Get a storage path for the given image URL. * Ensures the path will start with "uploads/images". diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php index 69b6dc90e..296e4d187 100644 --- a/tests/Uploads/ImageTest.php +++ b/tests/Uploads/ImageTest.php @@ -241,6 +241,36 @@ class ImageTest extends TestCase } } + public function test_secure_image_paths_traversal_causes_500() + { + config()->set('filesystems.images', 'local_secure'); + $this->asEditor(); + + $resp = $this->get('/uploads/images/../../logs/laravel.log'); + $resp->assertStatus(500); + } + + public function test_secure_image_paths_traversal_on_non_secure_images_causes_404() + { + config()->set('filesystems.images', 'local'); + $this->asEditor(); + + $resp = $this->get('/uploads/images/../../logs/laravel.log'); + $resp->assertStatus(404); + } + + public function test_secure_image_paths_dont_serve_non_images() + { + config()->set('filesystems.images', 'local_secure'); + $this->asEditor(); + + $testFilePath = storage_path('/uploads/images/testing.txt'); + file_put_contents($testFilePath, 'hello from test_secure_image_paths_dont_serve_non_images'); + + $resp = $this->get('/uploads/images/testing.txt'); + $resp->assertStatus(404); + } + public function test_secure_images_included_in_exports() { config()->set('filesystems.images', 'local_secure'); From c7fea8fe08e8d26840da7a0d353c05f7a84d3ff1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 00:24:42 +0000 Subject: [PATCH 06/10] Cleaned up logic within ImageRepo - Moved out extension check to ImageService as that seems more relevant. - Updated models to use static-style references instead of facade to align with common modern usage within the app. - Updated custom image_extension validation rule to use shared logic in image service. --- app/Entities/Tools/PageContent.php | 3 +- .../Controllers/Images/ImageController.php | 10 +----- .../CustomValidationServiceProvider.php | 6 ++-- app/Uploads/ImageRepo.php | 32 ++++--------------- app/Uploads/ImageService.php | 12 +++++++ 5 files changed, 25 insertions(+), 38 deletions(-) diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 724230a3d..c5b17ddef 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -9,6 +9,7 @@ use BookStack\Exceptions\ImageUploadException; use BookStack\Facades\Theme; use BookStack\Theming\ThemeEvents; use BookStack\Uploads\ImageRepo; +use BookStack\Uploads\ImageService; use BookStack\Util\HtmlContentFilter; use DOMDocument; use DOMNodeList; @@ -130,7 +131,7 @@ class PageContent $imageInfo = $this->parseBase64ImageUri($uri); // Validate extension and content - if (empty($imageInfo['data']) || !$imageRepo->imageExtensionSupported($imageInfo['extension'])) { + if (empty($imageInfo['data']) || !ImageService::isExtensionSupported($imageInfo['extension'])) { return ''; } diff --git a/app/Http/Controllers/Images/ImageController.php b/app/Http/Controllers/Images/ImageController.php index 66ccadc5e..231712d52 100644 --- a/app/Http/Controllers/Images/ImageController.php +++ b/app/Http/Controllers/Images/ImageController.php @@ -9,27 +9,19 @@ use BookStack\Uploads\Image; use BookStack\Uploads\ImageRepo; use BookStack\Uploads\ImageService; use Exception; -use Illuminate\Filesystem\Filesystem as File; -use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Storage; use Illuminate\Validation\ValidationException; -use League\Flysystem\Util; class ImageController extends Controller { - protected $image; - protected $file; protected $imageRepo; protected $imageService; /** * ImageController constructor. */ - public function __construct(Image $image, File $file, ImageRepo $imageRepo, ImageService $imageService) + public function __construct(ImageRepo $imageRepo, ImageService $imageService) { - $this->image = $image; - $this->file = $file; $this->imageRepo = $imageRepo; $this->imageService = $imageService; } diff --git a/app/Providers/CustomValidationServiceProvider.php b/app/Providers/CustomValidationServiceProvider.php index c54f48ca3..c2c0197c7 100644 --- a/app/Providers/CustomValidationServiceProvider.php +++ b/app/Providers/CustomValidationServiceProvider.php @@ -2,6 +2,7 @@ namespace BookStack\Providers; +use BookStack\Uploads\ImageService; use Illuminate\Support\Facades\Validator; use Illuminate\Support\ServiceProvider; @@ -13,9 +14,8 @@ class CustomValidationServiceProvider extends ServiceProvider public function boot(): void { Validator::extend('image_extension', function ($attribute, $value, $parameters, $validator) { - $validImageExtensions = ['png', 'jpg', 'jpeg', 'gif', 'webp']; - - return in_array(strtolower($value->getClientOriginalExtension()), $validImageExtensions); + $extension = strtolower($value->getClientOriginalExtension()); + return ImageService::isExtensionSupported($extension); }); Validator::extend('safe_url', function ($attribute, $value, $parameters, $validator) { diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 694560a14..4d46bb56d 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -11,36 +11,18 @@ use Symfony\Component\HttpFoundation\File\UploadedFile; class ImageRepo { - protected $image; protected $imageService; protected $restrictionService; - protected $page; - - protected static $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; /** * ImageRepo constructor. */ public function __construct( - Image $image, ImageService $imageService, - PermissionService $permissionService, - Page $page + PermissionService $permissionService ) { - $this->image = $image; $this->imageService = $imageService; $this->restrictionService = $permissionService; - $this->page = $page; - } - - /** - * Check if the given image extension is supported by BookStack. - * The extension must not be altered in this function. This check should provide a guarantee - * that the provided extension is safe to use for the image to be saved. - */ - public function imageExtensionSupported(string $extension): bool - { - return in_array($extension, static::$supportedExtensions); } /** @@ -48,7 +30,7 @@ class ImageRepo */ public function getById($id): Image { - return $this->image->findOrFail($id); + return Image::query()->findOrFail($id); } /** @@ -83,7 +65,7 @@ class ImageRepo string $search = null, callable $whereClause = null ): array { - $imageQuery = $this->image->newQuery()->where('type', '=', strtolower($type)); + $imageQuery = Image::query()->where('type', '=', strtolower($type)); if ($uploadedTo !== null) { $imageQuery = $imageQuery->where('uploaded_to', '=', $uploadedTo); @@ -114,7 +96,7 @@ class ImageRepo int $uploadedTo = null, string $search = null ): array { - $contextPage = $this->page->findOrFail($uploadedTo); + $contextPage = Page::visible()->findOrFail($uploadedTo); $parentFilter = null; if ($filterType === 'book' || $filterType === 'page') { @@ -205,7 +187,7 @@ class ImageRepo */ public function destroyByType(string $imageType) { - $images = $this->image->where('type', '=', $imageType)->get(); + $images = Image::query()->where('type', '=', $imageType)->get(); foreach ($images as $image) { $this->destroyImage($image); } @@ -218,10 +200,10 @@ class ImageRepo */ public function loadThumbs(Image $image) { - $image->thumbs = [ + $image->setAttribute('thumbs', [ 'gallery' => $this->getThumbnail($image, 150, 150, false), 'display' => $this->getThumbnail($image, 1680, null, true), - ]; + ]); } /** diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index dc18783c5..4cd818bcc 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -26,6 +26,8 @@ class ImageService protected $image; protected $fileSystem; + protected static $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; + /** * ImageService constructor. */ @@ -470,6 +472,16 @@ class ImageService return $disk->response($path); } + /** + * Check if the given image extension is supported by BookStack. + * The extension must not be altered in this function. This check should provide a guarantee + * that the provided extension is safe to use for the image to be saved. + */ + public static function isExtensionSupported(string $extension): bool + { + return in_array($extension, static::$supportedExtensions); + } + /** * Get a storage path for the given image URL. * Ensures the path will start with "uploads/images". From 4360da03d414cc926dde3d79e22e6e35f85838e1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 11:17:30 +0000 Subject: [PATCH 07/10] Ran a pass through image and attachment routes Added some stronger types, formatting changes and simplifications along the way. --- app/Actions/CommentRepo.php | 4 +- app/Http/Controllers/AttachmentController.php | 5 ++- .../Images/DrawioImageController.php | 5 +-- app/Uploads/ImageRepo.php | 29 +++++--------- app/Uploads/ImageService.php | 38 ++++++------------- 5 files changed, 28 insertions(+), 53 deletions(-) diff --git a/app/Actions/CommentRepo.php b/app/Actions/CommentRepo.php index 85fb6498a..8121dfc5c 100644 --- a/app/Actions/CommentRepo.php +++ b/app/Actions/CommentRepo.php @@ -66,13 +66,13 @@ class CommentRepo /** * Delete a comment from the system. */ - public function delete(Comment $comment) + public function delete(Comment $comment): void { $comment->delete(); } /** - * Convert the given comment markdown text to HTML. + * Convert the given comment Markdown to HTML. */ public function commentToHtml(string $commentText): string { diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php index 56503a694..477640b0a 100644 --- a/app/Http/Controllers/AttachmentController.php +++ b/app/Http/Controllers/AttachmentController.php @@ -68,6 +68,7 @@ class AttachmentController extends Controller 'file' => 'required|file', ]); + /** @var Attachment $attachment */ $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('view', $attachment->page); $this->checkOwnablePermission('page-update', $attachment->page); @@ -86,11 +87,10 @@ class AttachmentController extends Controller /** * Get the update form for an attachment. - * - * @return \Illuminate\Contracts\Foundation\Application|\Illuminate\Contracts\View\Factory|\Illuminate\View\View */ public function getUpdateForm(string $attachmentId) { + /** @var Attachment $attachment */ $attachment = Attachment::query()->findOrFail($attachmentId); $this->checkOwnablePermission('page-update', $attachment->page); @@ -173,6 +173,7 @@ class AttachmentController extends Controller /** * Get the attachments for a specific page. + * @throws NotFoundException */ public function listForPage(int $pageId) { diff --git a/app/Http/Controllers/Images/DrawioImageController.php b/app/Http/Controllers/Images/DrawioImageController.php index d99bb8e6f..b8e4546ff 100644 --- a/app/Http/Controllers/Images/DrawioImageController.php +++ b/app/Http/Controllers/Images/DrawioImageController.php @@ -67,13 +67,12 @@ class DrawioImageController extends Controller public function getAsBase64($id) { $image = $this->imageRepo->getById($id); - $page = $image->getPage(); - if ($image === null || $image->type !== 'drawio' || !userCan('page-view', $page)) { + if (is_null($image) || $image->type !== 'drawio' || !userCan('page-view', $image->getPage())) { return $this->jsonError('Image data could not be found'); } $imageData = $this->imageRepo->getImageData($image); - if ($imageData === null) { + if (is_null($imageData)) { return $this->jsonError('Image data could not be found'); } diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 4d46bb56d..67297f308 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -17,10 +17,7 @@ class ImageRepo /** * ImageRepo constructor. */ - public function __construct( - ImageService $imageService, - PermissionService $permissionService - ) { + public function __construct(ImageService $imageService, PermissionService $permissionService) { $this->imageService = $imageService; $this->restrictionService = $permissionService; } @@ -43,7 +40,7 @@ class ImageRepo $hasMore = count($images) > $pageSize; $returnImages = $images->take($pageSize); - $returnImages->each(function ($image) { + $returnImages->each(function (Image $image) { $this->loadThumbs($image); }); @@ -96,6 +93,7 @@ class ImageRepo int $uploadedTo = null, string $search = null ): array { + /** @var Page $contextPage */ $contextPage = Page::visible()->findOrFail($uploadedTo); $parentFilter = null; @@ -131,7 +129,7 @@ class ImageRepo * * @throws ImageUploadException */ - public function saveNewFromData(string $imageName, string $imageData, string $type, int $uploadedTo = 0) + public function saveNewFromData(string $imageName, string $imageData, string $type, int $uploadedTo = 0): Image { $image = $this->imageService->saveNew($imageName, $imageData, $type, $uploadedTo); $this->loadThumbs($image); @@ -140,13 +138,13 @@ class ImageRepo } /** - * Save a drawing the the database. + * Save a drawing in the database. * * @throws ImageUploadException */ public function saveDrawing(string $base64Uri, int $uploadedTo): Image { - $name = 'Drawing-' . strval(user()->id) . '-' . strval(time()) . '.png'; + $name = 'Drawing-' . user()->id . '-' . time() . '.png'; return $this->imageService->saveNewFromBase64Uri($base64Uri, $name, 'drawio', $uploadedTo); } @@ -154,7 +152,6 @@ class ImageRepo /** * Update the details of an image via an array of properties. * - * @throws ImageUploadException * @throws Exception */ public function updateImageDetails(Image $image, $updateDetails): Image @@ -171,13 +168,11 @@ class ImageRepo * * @throws Exception */ - public function destroyImage(Image $image = null): bool + public function destroyImage(Image $image = null): void { if ($image) { $this->imageService->destroy($image); } - - return true; } /** @@ -185,7 +180,7 @@ class ImageRepo * * @throws Exception */ - public function destroyByType(string $imageType) + public function destroyByType(string $imageType): void { $images = Image::query()->where('type', '=', $imageType)->get(); foreach ($images as $image) { @@ -195,10 +190,8 @@ class ImageRepo /** * Load thumbnails onto an image object. - * - * @throws Exception */ - public function loadThumbs(Image $image) + public function loadThumbs(Image $image): void { $image->setAttribute('thumbs', [ 'gallery' => $this->getThumbnail($image, 150, 150, false), @@ -210,10 +203,8 @@ class ImageRepo * Get the thumbnail for an image. * If $keepRatio is true only the width will be used. * Checks the cache then storage to avoid creating / accessing the filesystem on every check. - * - * @throws Exception */ - protected function getThumbnail(Image $image, ?int $width = 220, ?int $height = 220, bool $keepRatio = false): ?string + protected function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio): ?string { try { return $this->imageService->getThumbnail($image, $width, $height, $keepRatio); diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 4cd818bcc..eb2fc57b8 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -15,6 +15,8 @@ use Illuminate\Support\Str; use Intervention\Image\Exception\NotSupportedException; use Intervention\Image\ImageManager; use League\Flysystem\Util; +use Log; +use Psr\SimpleCache\InvalidArgumentException; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\StreamedResponse; @@ -156,7 +158,7 @@ class ImageService try { $this->saveImageDataInPublicSpace($storage, $this->adjustPathForStorageDisk($fullPath, $type), $imageData); } catch (Exception $e) { - \Log::error('Error when attempting image upload:' . $e->getMessage()); + Log::error('Error when attempting image upload:' . $e->getMessage()); throw new ImageUploadException(trans('errors.path_not_writable', ['filePath' => $fullPath])); } @@ -230,18 +232,10 @@ class ImageService * Get the thumbnail for an image. * If $keepRatio is true only the width will be used. * Checks the cache then storage to avoid creating / accessing the filesystem on every check. - * - * @param Image $image - * @param int $width - * @param int $height - * @param bool $keepRatio - * * @throws Exception - * @throws ImageUploadException - * - * @return string + * @throws InvalidArgumentException */ - public function getThumbnail(Image $image, $width = 220, $height = 220, $keepRatio = false) + public function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio = false): string { if ($keepRatio && $this->isGif($image)) { return $this->getPublicUrl($image->path); @@ -269,27 +263,16 @@ class ImageService } /** - * Resize image data. - * - * @param string $imageData - * @param int $width - * @param int $height - * @param bool $keepRatio + * Resize the image of given data to the specified size, and return the new image data. * * @throws ImageUploadException - * - * @return string */ - protected function resizeImage(string $imageData, $width = 220, $height = null, bool $keepRatio = true) + protected function resizeImage(string $imageData, ?int $width, ?int $height, bool $keepRatio): string { try { $thumb = $this->imageTool->make($imageData); - } catch (Exception $e) { - if ($e instanceof ErrorException || $e instanceof NotSupportedException) { - throw new ImageUploadException(trans('errors.cannot_create_thumbs')); - } - - throw $e; + } catch (ErrorException | NotSupportedException $e) { + throw new ImageUploadException(trans('errors.cannot_create_thumbs')); } if ($keepRatio) { @@ -523,7 +506,7 @@ class ImageService */ private function getPublicUrl(string $filePath): string { - if ($this->storageUrl === null) { + if (is_null($this->storageUrl)) { $storageUrl = config('filesystems.url'); // Get the standard public s3 url if s3 is set as storage type @@ -537,6 +520,7 @@ class ImageService $storageUrl = 'https://s3-' . $storageDetails['region'] . '.amazonaws.com/' . $storageDetails['bucket']; } } + $this->storageUrl = $storageUrl; } From bfbccbede14853c68edecf5dd5d08a50a6ed5c9d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 11:32:00 +0000 Subject: [PATCH 08/10] Updated attachments to not be saved with a complete extension Intended to limit impact in the event the storage path is potentially exposed. --- app/Auth/User.php | 2 +- app/Uploads/AttachmentService.php | 2 +- app/Uploads/ImageService.php | 2 +- tests/Uploads/AttachmentTest.php | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/Auth/User.php b/app/Auth/User.php index 0a6849fe0..da47a9d69 100644 --- a/app/Auth/User.php +++ b/app/Auth/User.php @@ -27,7 +27,7 @@ use Illuminate\Support\Collection; /** * Class User. * - * @property string $id + * @property int $id * @property string $name * @property string $slug * @property string $email diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php index c9cd99b38..52954d24f 100644 --- a/app/Uploads/AttachmentService.php +++ b/app/Uploads/AttachmentService.php @@ -216,7 +216,7 @@ class AttachmentService $storage = $this->getStorageDisk(); $basePath = 'uploads/files/' . date('Y-m-M') . '/'; - $uploadFileName = Str::random(16) . '.' . $uploadedFile->getClientOriginalExtension(); + $uploadFileName = Str::random(16) . '-' . $uploadedFile->getClientOriginalExtension(); while ($storage->exists($this->adjustPathForStorageDisk($basePath . $uploadFileName))) { $uploadFileName = Str::random(3) . $uploadFileName; } diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index eb2fc57b8..0c3dfc47d 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -11,11 +11,11 @@ use Illuminate\Contracts\Filesystem\FileNotFoundException; use Illuminate\Contracts\Filesystem\Filesystem as FileSystemInstance; use Illuminate\Contracts\Filesystem\Filesystem as Storage; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; use Intervention\Image\Exception\NotSupportedException; use Intervention\Image\ImageManager; use League\Flysystem\Util; -use Log; use Psr\SimpleCache\InvalidArgumentException; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\StreamedResponse; diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 26f092bcc..1682577bf 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -109,7 +109,8 @@ class AttachmentTest extends TestCase $attachment = Attachment::query()->orderBy('id', 'desc')->first(); $this->assertStringNotContainsString($fileName, $attachment->path); - $this->assertStringEndsWith('.txt', $attachment->path); + $this->assertStringEndsWith('-txt', $attachment->path); + $this->deleteUploads(); } public function test_file_display_and_access() From f4201e574044e839c41af6893b48884b1b727a45 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 13:16:15 +0000 Subject: [PATCH 09/10] New Crowdin updates (#3023) * New translations errors.php (Polish) * New translations activities.php (Dutch) * New translations auth.php (Dutch) * New translations common.php (Dutch) * New translations entities.php (Dutch) * New translations auth.php (Dutch) * New translations auth.php (Dutch) * New translations auth.php (Dutch) * New translations settings.php (Latvian) --- resources/lang/lv/settings.php | 2 +- resources/lang/nl/activities.php | 4 ++-- resources/lang/nl/auth.php | 40 ++++++++++++++++---------------- resources/lang/nl/common.php | 2 +- resources/lang/nl/entities.php | 4 ++-- resources/lang/pl/errors.php | 6 ++--- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/resources/lang/lv/settings.php b/resources/lang/lv/settings.php index 480fe0144..d3144412f 100644 --- a/resources/lang/lv/settings.php +++ b/resources/lang/lv/settings.php @@ -248,7 +248,7 @@ return [ 'de_informal' => 'Deutsch (Du)', 'es' => 'Español', 'es_AR' => 'Español Argentina', - 'et' => 'Eesti keel', + 'et' => 'Igauņu', 'fr' => 'Français', 'he' => 'עברית', 'hr' => 'Hrvatski', diff --git a/resources/lang/nl/activities.php b/resources/lang/nl/activities.php index f45ea074a..3a3b36a87 100644 --- a/resources/lang/nl/activities.php +++ b/resources/lang/nl/activities.php @@ -48,8 +48,8 @@ return [ 'favourite_remove_notification' => '":name" is verwijderd uit je favorieten', // MFA - 'mfa_setup_method_notification' => 'Multi-factor method successfully configured', - 'mfa_remove_method_notification' => 'Multi-factor method successfully removed', + 'mfa_setup_method_notification' => 'Multi-factor methode succesvol geconfigureerd', + 'mfa_remove_method_notification' => 'Multi-factor methode succesvol verwijderd', // Other 'commented_on' => 'reageerde op', diff --git a/resources/lang/nl/auth.php b/resources/lang/nl/auth.php index f57b2ecc7..bbfe5f0e2 100644 --- a/resources/lang/nl/auth.php +++ b/resources/lang/nl/auth.php @@ -76,27 +76,27 @@ return [ 'user_invite_success' => 'Wachtwoord ingesteld, je hebt nu toegang tot :appName!', // Multi-factor Authentication - 'mfa_setup' => 'Setup Multi-Factor Authentication', - 'mfa_setup_desc' => 'Setup multi-factor authentication as an extra layer of security for your user account.', - 'mfa_setup_configured' => 'Already configured', - 'mfa_setup_reconfigure' => 'Reconfigure', - 'mfa_setup_remove_confirmation' => 'Are you sure you want to remove this multi-factor authentication method?', - 'mfa_setup_action' => 'Setup', - 'mfa_backup_codes_usage_limit_warning' => 'You have less than 5 backup codes remaining, Please generate and store a new set before you run out of codes to prevent being locked out of your account.', - 'mfa_option_totp_title' => 'Mobile App', - 'mfa_option_totp_desc' => 'To use multi-factor authentication you\'ll need a mobile application that supports TOTP such as Google Authenticator, Authy or Microsoft Authenticator.', - 'mfa_option_backup_codes_title' => 'Backup Codes', - 'mfa_option_backup_codes_desc' => 'Securely store a set of one-time-use backup codes which you can enter to verify your identity.', - 'mfa_gen_confirm_and_enable' => 'Confirm and Enable', - 'mfa_gen_backup_codes_title' => 'Backup Codes Setup', - 'mfa_gen_backup_codes_desc' => 'Store the below list of codes in a safe place. When accessing the system you\'ll be able to use one of the codes as a second authentication mechanism.', + 'mfa_setup' => 'Multi-factor authenticatie instellen', + 'mfa_setup_desc' => 'Stel multi-factor authenticatie in als een extra beveiligingslaag voor uw gebruikersaccount.', + 'mfa_setup_configured' => 'Reeds geconfigureerd', + 'mfa_setup_reconfigure' => 'Herconfigureren1', + 'mfa_setup_remove_confirmation' => 'Weet je zeker dat je deze multi-factor authenticatie methode wilt verwijderen?', + 'mfa_setup_action' => 'Instellen', + 'mfa_backup_codes_usage_limit_warning' => 'U heeft minder dan 5 back-upcodes resterend. Genereer en sla een nieuwe set op voordat je geen codes meer hebt om te voorkomen dat je buiten je account wordt gesloten.', + 'mfa_option_totp_title' => 'Mobiele app', + 'mfa_option_totp_desc' => 'Om multi-factor authenticatie te gebruiken heeft u een mobiele applicatie nodig die TOTP ondersteunt, zoals Google Authenticator, Authy of Microsoft Authenticator.', + 'mfa_option_backup_codes_title' => 'Back-up Codes', + 'mfa_option_backup_codes_desc' => 'Bewaar veilig een set eenmalige back-upcodes die u kunt invoeren om uw identiteit te verifiëren.', + 'mfa_gen_confirm_and_enable' => 'Bevestigen en inschakelen', + 'mfa_gen_backup_codes_title' => 'Reservekopiecodes instellen', + 'mfa_gen_backup_codes_desc' => 'De onderstaande lijst met codes opslaan op een veilige plaats. Bij de toegang tot het systeem kun je een van de codes gebruiken als tweede verificatiemechanisme.', 'mfa_gen_backup_codes_download' => 'Download Codes', - 'mfa_gen_backup_codes_usage_warning' => 'Each code can only be used once', - 'mfa_gen_totp_title' => 'Mobile App Setup', - 'mfa_gen_totp_desc' => 'To use multi-factor authentication you\'ll need a mobile application that supports TOTP such as Google Authenticator, Authy or Microsoft Authenticator.', - 'mfa_gen_totp_scan' => 'Scan the QR code below using your preferred authentication app to get started.', - 'mfa_gen_totp_verify_setup' => 'Verify Setup', - 'mfa_gen_totp_verify_setup_desc' => 'Verify that all is working by entering a code, generated within your authentication app, in the input box below:', + 'mfa_gen_backup_codes_usage_warning' => 'Elke code kan slechts eenmaal gebruikt worden', + 'mfa_gen_totp_title' => 'Mobiele app installatie', + 'mfa_gen_totp_desc' => 'Om multi-factor authenticatie te gebruiken heeft u een mobiele applicatie nodig die TOTP ondersteunt, zoals Google Authenticator, Authy of Microsoft Authenticator.', + 'mfa_gen_totp_scan' => 'Scan de onderstaande QR-code door gebruik te maken van uw favoriete authenticatie app om aan de slag te gaan.', + 'mfa_gen_totp_verify_setup' => 'Installatie verifiëren', + 'mfa_gen_totp_verify_setup_desc' => 'Controleer of alles werkt door het invoeren van een code, die wordt gegenereerd binnen uw authenticatie-app, in het onderstaande invoerveld:', 'mfa_gen_totp_provide_code_here' => 'Provide your app generated code here', 'mfa_verify_access' => 'Verify Access', 'mfa_verify_access_desc' => 'Your user account requires you to confirm your identity via an additional level of verification before you\'re granted access. Verify using one of your configured methods to continue.', diff --git a/resources/lang/nl/common.php b/resources/lang/nl/common.php index c53df6f2c..352e905aa 100644 --- a/resources/lang/nl/common.php +++ b/resources/lang/nl/common.php @@ -39,7 +39,7 @@ return [ 'reset' => 'Resetten', 'remove' => 'Verwijderen', 'add' => 'Toevoegen', - 'configure' => 'Configure', + 'configure' => 'Configureer', 'fullscreen' => 'Volledig scherm', 'favourite' => 'Favoriet', 'unfavourite' => 'Verwijderen uit favoriet', diff --git a/resources/lang/nl/entities.php b/resources/lang/nl/entities.php index e08ae3e62..d22df4123 100644 --- a/resources/lang/nl/entities.php +++ b/resources/lang/nl/entities.php @@ -99,7 +99,7 @@ return [ 'shelves_permissions' => 'Boekenplank permissies', 'shelves_permissions_updated' => 'Boekenplank permissies opgeslagen', 'shelves_permissions_active' => 'Boekenplank permissies actief', - 'shelves_permissions_cascade_warning' => 'Permissions on bookshelves do not automatically cascade to contained books. This is because a book can exist on multiple shelves. Permissions can however be copied down to child books using the option found below.', + 'shelves_permissions_cascade_warning' => 'Machtigingen op boekenplanken zijn niet automatisch een cascade om boeken te bevatten. Dit komt omdat een boek in meerdere schappen kan bestaan. Machtigingen kunnen echter worden gekopieerd naar subboeken door gebruik te maken van onderstaande optie.', 'shelves_copy_permissions_to_books' => 'Kopieer permissies naar boeken', 'shelves_copy_permissions' => 'Kopieer permissies', 'shelves_copy_permissions_explain' => 'Met deze actie worden de permissies van deze boekenplank gekopieërd naar alle boeken op de plank. Voordat deze actie wordt uitgevoerd, zorg dat de wijzigingen in de permissies van deze boekenplank zijn opgeslagen.', @@ -234,7 +234,7 @@ return [ 'pages_initial_name' => 'Nieuwe pagina', 'pages_editing_draft_notification' => 'U bewerkt momenteel een concept dat voor het laatst is opgeslagen op :timeDiff.', 'pages_draft_edited_notification' => 'Deze pagina is sindsdien bijgewerkt. Het wordt aanbevolen dat u dit concept verwijderd.', - 'pages_draft_page_changed_since_creation' => 'This page has been updated since this draft was created. It is recommended that you discard this draft or take care not to overwrite any page changes.', + 'pages_draft_page_changed_since_creation' => 'Deze pagina is bijgewerkt sinds het aanmaken van dit concept. Het wordt aanbevolen dat u dit ontwerp verwijdert of ervoor zorgt dat u wijzigingen op de pagina niet overschrijft.', 'pages_draft_edit_active' => [ 'start_a' => ':count gebruikers zijn begonnen deze pagina te bewerken', 'start_b' => ':userName is begonnen met het bewerken van deze pagina', diff --git a/resources/lang/pl/errors.php b/resources/lang/pl/errors.php index 8d3ce30ed..e493705b5 100644 --- a/resources/lang/pl/errors.php +++ b/resources/lang/pl/errors.php @@ -24,9 +24,9 @@ return [ 'saml_invalid_response_id' => 'Żądanie z zewnętrznego systemu uwierzytelniania nie zostało rozpoznane przez proces rozpoczęty przez tę aplikację. Cofnięcie po zalogowaniu mogło spowodować ten problem.', 'saml_fail_authed' => 'Logowanie przy użyciu :system nie powiodło się, system nie mógł pomyślnie ukończyć uwierzytelniania', 'oidc_already_logged_in' => 'Już zalogowany', - 'oidc_user_not_registered' => 'The user :name is not registered and automatic registration is disabled', - 'oidc_no_email_address' => 'Could not find an email address, for this user, in the data provided by the external authentication system', - 'oidc_fail_authed' => 'Login using :system failed, system did not provide successful authorization', + 'oidc_user_not_registered' => 'Użytkownik :name nie jest zarejestrowany, a automatyczna rejestracja jest wyłączona', + 'oidc_no_email_address' => 'Nie można odnaleźć adresu email dla tego użytkownika w danych dostarczonych przez zewnętrzny system uwierzytelniania', + 'oidc_fail_authed' => 'Logowanie przy użyciu :system nie powiodło się, system nie mógł pomyślnie ukończyć uwierzytelniania', 'social_no_action_defined' => 'Brak zdefiniowanej akcji', 'social_login_bad_response' => "Podczas próby logowania :socialAccount wystąpił błąd: \n:error", 'social_account_in_use' => 'To konto :socialAccount jest już w użyciu. Spróbuj zalogować się za pomocą opcji :socialAccount.', From a17be959d8b148f439305498ada0babd1b0ca2c7 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 13:26:02 +0000 Subject: [PATCH 10/10] Applied latest styleci changes --- app/Entities/Tools/PageContent.php | 4 +++- app/Http/Controllers/AttachmentController.php | 1 + app/Http/Controllers/Controller.php | 4 +--- app/Providers/CustomValidationServiceProvider.php | 1 + app/Uploads/ImageRepo.php | 3 ++- app/Uploads/ImageService.php | 4 +++- app/Util/WebSafeMimeSniffer.php | 4 +--- tests/Auth/MfaVerificationTest.php | 4 ++-- tests/Entity/PageContentTest.php | 1 - tests/Uploads/AttachmentTest.php | 3 +-- 10 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index c5b17ddef..b1323bc68 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -149,15 +149,17 @@ class PageContent /** * Parse a base64 image URI into the data and extension. + * * @return array{extension: array, data: string} */ protected function parseBase64ImageUri(string $uri): array { [$dataDefinition, $base64ImageData] = explode(',', $uri, 2); $extension = strtolower(preg_split('/[\/;]/', $dataDefinition)[1] ?? ''); + return [ 'extension' => $extension, - 'data' => base64_decode($base64ImageData) ?: '', + 'data' => base64_decode($base64ImageData) ?: '', ]; } diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php index 477640b0a..c08247dad 100644 --- a/app/Http/Controllers/AttachmentController.php +++ b/app/Http/Controllers/AttachmentController.php @@ -173,6 +173,7 @@ class AttachmentController extends Controller /** * Get the attachments for a specific page. + * * @throws NotFoundException */ public function listForPage(int $pageId) diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 3bccdcda4..d63280a23 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -6,7 +6,6 @@ use BookStack\Facades\Activity; use BookStack\Interfaces\Loggable; use BookStack\Model; use BookStack\Util\WebSafeMimeSniffer; -use finfo; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Foundation\Validation\ValidatesRequests; use Illuminate\Http\Exceptions\HttpResponseException; @@ -130,8 +129,7 @@ abstract class Controller extends BaseController */ protected function inlineDownloadResponse(string $content, string $fileName): Response { - - $mime = (new WebSafeMimeSniffer)->sniff($content); + $mime = (new WebSafeMimeSniffer())->sniff($content); return response()->make($content, 200, [ 'Content-Type' => $mime, diff --git a/app/Providers/CustomValidationServiceProvider.php b/app/Providers/CustomValidationServiceProvider.php index c2c0197c7..ac95099cc 100644 --- a/app/Providers/CustomValidationServiceProvider.php +++ b/app/Providers/CustomValidationServiceProvider.php @@ -15,6 +15,7 @@ class CustomValidationServiceProvider extends ServiceProvider { Validator::extend('image_extension', function ($attribute, $value, $parameters, $validator) { $extension = strtolower($value->getClientOriginalExtension()); + return ImageService::isExtensionSupported($extension); }); diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 67297f308..5c6228b37 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -17,7 +17,8 @@ class ImageRepo /** * ImageRepo constructor. */ - public function __construct(ImageService $imageService, PermissionService $permissionService) { + public function __construct(ImageService $imageService, PermissionService $permissionService) + { $this->imageService = $imageService; $this->restrictionService = $permissionService; } diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 0c3dfc47d..644269731 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -232,6 +232,7 @@ class ImageService * Get the thumbnail for an image. * If $keepRatio is true only the width will be used. * Checks the cache then storage to avoid creating / accessing the filesystem on every check. + * * @throws Exception * @throws InvalidArgumentException */ @@ -271,7 +272,7 @@ class ImageService { try { $thumb = $this->imageTool->make($imageData); - } catch (ErrorException | NotSupportedException $e) { + } catch (ErrorException|NotSupportedException $e) { throw new ImageUploadException(trans('errors.cannot_create_thumbs')); } @@ -452,6 +453,7 @@ class ImageService public function streamImageFromStorageResponse(string $imageType, string $path): StreamedResponse { $disk = $this->getStorageDisk($imageType); + return $disk->response($path); } diff --git a/app/Util/WebSafeMimeSniffer.php b/app/Util/WebSafeMimeSniffer.php index a896bd9e5..4012004fc 100644 --- a/app/Util/WebSafeMimeSniffer.php +++ b/app/Util/WebSafeMimeSniffer.php @@ -10,7 +10,6 @@ use finfo; */ class WebSafeMimeSniffer { - /** * @var string[] */ @@ -61,5 +60,4 @@ class WebSafeMimeSniffer return 'application/octet-stream'; } - -} \ No newline at end of file +} diff --git a/tests/Auth/MfaVerificationTest.php b/tests/Auth/MfaVerificationTest.php index ee6f3ecc8..49ca6663d 100644 --- a/tests/Auth/MfaVerificationTest.php +++ b/tests/Auth/MfaVerificationTest.php @@ -242,7 +242,7 @@ class MfaVerificationTest extends TestCase } /** - * @return Array + * @return array */ protected function startTotpLogin(): array { @@ -260,7 +260,7 @@ class MfaVerificationTest extends TestCase } /** - * @return Array + * @return array */ protected function startBackupCodeLogin($codes = ['kzzu6-1pgll', 'bzxnf-plygd', 'bwdsp-ysl51', '1vo93-ioy7n', 'lf7nw-wdyka', 'xmtrd-oplac']): array { diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index 049b47f0e..d3a00ebde 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -614,7 +614,6 @@ class PageContentTest extends TestCase $page->refresh(); $this->assertStringContainsString('html); } - } public function test_base64_images_get_extracted_from_markdown_page_content() diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 1682577bf..abd7ca616 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/tests/Uploads/AttachmentTest.php @@ -9,7 +9,6 @@ use BookStack\Uploads\Attachment; use BookStack\Uploads\AttachmentService; use Illuminate\Http\UploadedFile; use Tests\TestCase; -use Tests\TestResponse; class AttachmentTest extends TestCase { @@ -56,6 +55,7 @@ class AttachmentTest extends TestCase $upload = new UploadedFile($filePath, $filename, $mimeType, null, true); $this->call('POST', '/attachments/upload', ['uploaded_to' => $page->id], [], ['file' => $upload], []); + return $page->attachments()->latest()->firstOrFail(); } @@ -340,5 +340,4 @@ class AttachmentTest extends TestCase $this->deleteUploads(); } - }