starred/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2026-07-15 21:31:36 +03:00
Code Issues 784 Packages Projects Releases 25 Wiki Activity

Attachments: Moved perm checks before validation

Browse Source 
Avoids providing responses with potential sensitive attachment info
before permission checks.
Added tests to cover.

Thanks to Rafael Castilho for reporting.
This commit is contained in:
Dan Brown
2026-06-07 10:22:52 +01:00
parent 81f77a95de
commit b7325fdf0e
2 changed files with 39 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
app/Uploads/Controllers/AttachmentController.php
View File

@@ -107,6 +107,9 @@ class AttachmentController extends Controller
{
/** @var Attachment $attachment */
$attachment = Attachment::query()->findOrFail($attachmentId);
$this->checkOwnablePermission(Permission::PageView, $attachment->page);
$this->checkOwnablePermission(Permission::PageUpdate, $attachment->page);
$this->checkOwnablePermission(Permission::AttachmentUpdate, $attachment);
try {
$this->validate($request, [
@@ -120,10 +123,6 @@ class AttachmentController extends Controller
]), 422);
}
$this->checkOwnablePermission(Permission::PageView, $attachment->page);
$this->checkOwnablePermission(Permission::PageUpdate, $attachment->page);
$this->checkOwnablePermission(Permission::AttachmentUpdate, $attachment);
$attachment = $this->attachmentService->updateFile($attachment, [
'name' => $request->input('attachment_edit_name'),
'link' => $request->input('attachment_edit_url'),
@@ -142,6 +141,10 @@ class AttachmentController extends Controller
public function attachLink(Request $request)
{
$pageId = $request->input('attachment_link_uploaded_to');
$page = $this->pageQueries->findVisibleByIdOrFail($pageId);
$this->checkPermission(Permission::AttachmentCreateAll);
$this->checkOwnablePermission(Permission::PageUpdate, $page);
try {
$this->validate($request, [
@@ -156,11 +159,6 @@ class AttachmentController extends Controller
]), 422);
}
$page = $this->pageQueries->findVisibleByIdOrFail($pageId);
$this->checkPermission(Permission::AttachmentCreateAll);
$this->checkOwnablePermission(Permission::PageUpdate, $page);
$attachmentName = $request->input('attachment_link_name');
$link = $request->input('attachment_link_url');
$this->attachmentService->saveNewFromLink($attachmentName, $link, intval($pageId));

32
tests/Uploads/AttachmentTest.php
View File

@@ -159,6 +159,38 @@ class AttachmentTest extends TestCase
$this->files->deleteAllAttachmentFiles();
}
public function test_attachment_update_without_permission()
{
$page = $this->entities->page();
$attachment = Attachment::factory()->create(['uploaded_to' => $page->id]);
$this->permissions->disableEntityInheritedPermissions($page);
$resp = $this->asViewer()->put("attachments/{$attachment->id}", [
'attachment_edit_name' => 'My new attachment name',
'attachment_edit_url' => 'https://test.example.com',
]);
$this->assertPermissionError($resp);
}
public function test_attachment_update_without_permission_with_validation_errors()
{
$page = $this->entities->page();
/** @var Attachment $attachment */
$attachment = Attachment::factory()->create(['uploaded_to' => $page->id]);
$this->permissions->disableEntityInheritedPermissions($page);
$resp = $this->asViewer()->put("attachments/{$attachment->id}", [
'attachment_edit_name' => '',
'attachment_edit_url' => 'https://test.example.com',
]);
$this->assertPermissionError($resp);
$resp->assertDontSee($attachment->path);
}
public function test_file_deletion()
{
$page = $this->entities->page();