From 85154fff693c80374bc048ca9497df663bd5e32d Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 14 Nov 2021 22:03:22 +0000
Subject: [PATCH] Added an env configurable file upload size limit

Replaces the old suggestion of setting JS head 'window.uploadLimit'
variable. This new env option will be used by back-end validation and
front-end libs/logic too.

Limits already likely exist within prod environments at a PHP and
webserver level but this allows an app-level limit and centralises the
option on the BookStack side into the .env

Closes #3033
---
 .env.example.complete                         |  4 ++
 app/Config/app.php                            |  3 ++
 app/Entities/Tools/PageContent.php            |  6 +++
 app/Http/Controllers/Api/ApiController.php    |  5 +++
 .../Api/AttachmentApiController.php           | 37 ++++++++++---------
 app/Http/Controllers/AttachmentController.php |  5 ++-
 app/Http/Controllers/Controller.php           |  4 +-
 app/Uploads/AttachmentService.php             |  8 ++++
 resources/js/components/dropzone.js           |  3 +-
 resources/views/form/dropzone.blade.php       |  1 +
 10 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/.env.example.complete b/.env.example.complete
index 683db703c..9a46b23a5 100644
--- a/.env.example.complete
+++ b/.env.example.complete
@@ -293,6 +293,10 @@ REVISION_LIMIT=50
 # Set to -1 for unlimited recycle bin lifetime.
 RECYCLE_BIN_LIFETIME=30
 
+# File Upload Limit
+# Maximum file size, in megabytes, that can be uploaded to the system.
+FILE_UPLOAD_SIZE_LIMIT=50
+
 # Allow <script> tags in page content
 # Note, if set to 'true' the page editor may still escape scripts.
 ALLOW_CONTENT_SCRIPTS=false
diff --git a/app/Config/app.php b/app/Config/app.php
index 44e382cdc..39bfa7134 100644
--- a/app/Config/app.php
+++ b/app/Config/app.php
@@ -31,6 +31,9 @@ return [
     // Set to -1 for unlimited recycle bin lifetime.
     'recycle_bin_lifetime' => env('RECYCLE_BIN_LIFETIME', 30),
 
+    // The limit for all uploaded files, including images and attachments in MB.
+    'upload_limit' => env('FILE_UPLOAD_SIZE_LIMIT', 50),
+
     // Allow <script> tags to entered within page content.
     // <script> tags are escaped by default.
     // Even when overridden the WYSIWYG editor may still escape script content.
diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php
index b1323bc68..52a10ae87 100644
--- a/app/Entities/Tools/PageContent.php
+++ b/app/Entities/Tools/PageContent.php
@@ -135,6 +135,12 @@ class PageContent
             return '';
         }
 
+        // Validate that the content is not over our upload limit
+        $uploadLimitBytes = (config('app.upload_limit') * 1000000);
+        if (strlen($imageInfo['data']) > $uploadLimitBytes) {
+            return '';
+        }
+
         // Save image from data with a random name
         $imageName = 'embedded-image-' . Str::random(8) . '.' . $imageInfo['extension'];
 
diff --git a/app/Http/Controllers/Api/ApiController.php b/app/Http/Controllers/Api/ApiController.php
index fc9788b06..3f049a08c 100644
--- a/app/Http/Controllers/Api/ApiController.php
+++ b/app/Http/Controllers/Api/ApiController.php
@@ -24,9 +24,14 @@ abstract class ApiController extends Controller
 
     /**
      * Get the validation rules for this controller.
+     * Defaults to a $rules property but can be a rules() method.
      */
     public function getValdationRules(): array
     {
+        if (method_exists($this, 'rules')) {
+            return $this->rules();
+        }
+
         return $this->rules;
     }
 }
diff --git a/app/Http/Controllers/Api/AttachmentApiController.php b/app/Http/Controllers/Api/AttachmentApiController.php
index 67815bc47..fc5008e3c 100644
--- a/app/Http/Controllers/Api/AttachmentApiController.php
+++ b/app/Http/Controllers/Api/AttachmentApiController.php
@@ -15,21 +15,6 @@ class AttachmentApiController extends ApiController
 {
     protected $attachmentService;
 
-    protected $rules = [
-        'create' => [
-            'name'        => ['required', 'min:1', 'max:255', 'string'],
-            'uploaded_to' => ['required', 'integer', 'exists:pages,id'],
-            'file'        => ['required_without:link', 'file'],
-            'link'        => ['required_without:file', 'min:1', 'max:255', 'safe_url'],
-        ],
-        'update' => [
-            'name'        => ['min:1', 'max:255', 'string'],
-            'uploaded_to' => ['integer', 'exists:pages,id'],
-            'file'        => ['file'],
-            'link'        => ['min:1', 'max:255', 'safe_url'],
-        ],
-    ];
-
     public function __construct(AttachmentService $attachmentService)
     {
         $this->attachmentService = $attachmentService;
@@ -61,7 +46,7 @@ class AttachmentApiController extends ApiController
     public function create(Request $request)
     {
         $this->checkPermission('attachment-create-all');
-        $requestData = $this->validate($request, $this->rules['create']);
+        $requestData = $this->validate($request, $this->rules()['create']);
 
         $pageId = $request->get('uploaded_to');
         $page = Page::visible()->findOrFail($pageId);
@@ -122,7 +107,7 @@ class AttachmentApiController extends ApiController
      */
     public function update(Request $request, string $id)
     {
-        $requestData = $this->validate($request, $this->rules['update']);
+        $requestData = $this->validate($request, $this->rules()['update']);
         /** @var Attachment $attachment */
         $attachment = Attachment::visible()->findOrFail($id);
 
@@ -162,4 +147,22 @@ class AttachmentApiController extends ApiController
 
         return response('', 204);
     }
+
+    protected function rules(): array
+    {
+        return [
+            'create' => [
+                'name'        => ['required', 'min:1', 'max:255', 'string'],
+                'uploaded_to' => ['required', 'integer', 'exists:pages,id'],
+                'file'        => array_merge(['required_without:link'], $this->attachmentService->getFileValidationRules()),
+                'link'        => ['required_without:file', 'min:1', 'max:255', 'safe_url'],
+            ],
+            'update' => [
+                'name'        => ['min:1', 'max:255', 'string'],
+                'uploaded_to' => ['integer', 'exists:pages,id'],
+                'file'        => $this->attachmentService->getFileValidationRules(),
+                'link'        => ['min:1', 'max:255', 'safe_url'],
+            ],
+        ];
+    }
 }
diff --git a/app/Http/Controllers/AttachmentController.php b/app/Http/Controllers/AttachmentController.php
index ec9872bcf..445611fcb 100644
--- a/app/Http/Controllers/AttachmentController.php
+++ b/app/Http/Controllers/AttachmentController.php
@@ -9,6 +9,7 @@ use BookStack\Uploads\Attachment;
 use BookStack\Uploads\AttachmentService;
 use Exception;
 use Illuminate\Contracts\Filesystem\FileNotFoundException;
+use Illuminate\Foundation\Http\Middleware\ValidatePostSize;
 use Illuminate\Http\Request;
 use Illuminate\Support\MessageBag;
 use Illuminate\Validation\ValidationException;
@@ -37,7 +38,7 @@ class AttachmentController extends Controller
     {
         $this->validate($request, [
             'uploaded_to' => ['required', 'integer', 'exists:pages,id'],
-            'file'        => ['required', 'file'],
+            'file'        => array_merge(['required'], $this->attachmentService->getFileValidationRules()),
         ]);
 
         $pageId = $request->get('uploaded_to');
@@ -65,7 +66,7 @@ class AttachmentController extends Controller
     public function uploadUpdate(Request $request, $attachmentId)
     {
         $this->validate($request, [
-            'file' => ['required', 'file'],
+            'file' => array_merge(['required'], $this->attachmentService->getFileValidationRules()),
         ]);
 
         /** @var Attachment $attachment */
diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php
index 047e0bed9..f836f18ed 100644
--- a/app/Http/Controllers/Controller.php
+++ b/app/Http/Controllers/Controller.php
@@ -165,7 +165,7 @@ abstract class Controller extends BaseController
     /**
      * Log an activity in the system.
      *
-     * @param $detail string|Loggable
+     * @param string|Loggable $detail
      */
     protected function logActivity(string $type, $detail = ''): void
     {
@@ -177,6 +177,6 @@ abstract class Controller extends BaseController
      */
     protected function getImageValidationRules(): array
     {
-        return ['image_extension', 'mimes:jpeg,png,gif,webp'];
+        return ['image_extension', 'mimes:jpeg,png,gif,webp', 'max:' . (config('app.upload_limit') * 1000)];
     }
 }
diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php
index e62a18c82..7974d7ae9 100644
--- a/app/Uploads/AttachmentService.php
+++ b/app/Uploads/AttachmentService.php
@@ -233,4 +233,12 @@ class AttachmentService
 
         return $attachmentPath;
     }
+
+    /**
+     * Get the file validation rules for attachments.
+     */
+    public function getFileValidationRules(): array
+    {
+        return ['file', 'max:' . (config('app.upload_limit') * 1000)];
+    }
 }
diff --git a/resources/js/components/dropzone.js b/resources/js/components/dropzone.js
index e7273df62..44fdf2d0d 100644
--- a/resources/js/components/dropzone.js
+++ b/resources/js/components/dropzone.js
@@ -11,6 +11,7 @@ class Dropzone {
         this.url = this.$opts.url;
         this.successMessage = this.$opts.successMessage;
         this.removeMessage = this.$opts.removeMessage;
+        this.uploadLimit = Number(this.$opts.uploadLimit);
         this.uploadLimitMessage = this.$opts.uploadLimitMessage;
         this.timeoutMessage = this.$opts.timeoutMessage;
 
@@ -19,7 +20,7 @@ class Dropzone {
             addRemoveLinks: true,
             dictRemoveFile: this.removeMessage,
             timeout: Number(window.uploadTimeout) || 60000,
-            maxFilesize: Number(window.uploadLimit) || 256,
+            maxFilesize: this.uploadLimit,
             url: this.url,
             withCredentials: true,
             init() {
diff --git a/resources/views/form/dropzone.blade.php b/resources/views/form/dropzone.blade.php
index 6c5ac4929..118761d4c 100644
--- a/resources/views/form/dropzone.blade.php
+++ b/resources/views/form/dropzone.blade.php
@@ -7,6 +7,7 @@
      option:dropzone:url="{{ $url }}"
      option:dropzone:success-message="{{ $successMessage ?? '' }}"
      option:dropzone:remove-message="{{ trans('components.image_upload_remove') }}"
+     option:dropzone:upload-limit="{{ config('app.upload_limit') }}"
      option:dropzone:upload-limit-message="{{ trans('errors.server_upload_limit') }}"
      option:dropzone:timeout-message="{{ trans('errors.file_upload_timeout') }}"