From d38b99887d001b05357dc3131356f3f4aeceaa83 Mon Sep 17 00:00:00 2001
From: Erik Michelson <github@erik.michelson.eu>
Date: Mon, 18 May 2026 21:41:47 +0200
Subject: [PATCH] fix(revisions): always check noteId when fetching a revision

This fixes a reported security vulnerability where one use could
retrieve revisions of another note where they don't have access
to. This was possible, because the URL included both the note alias
and the revision UUID, the backend then checked the user's permissions
for the note alias but fetched and returned the revision by its UUID
without checking whether the revision belongs to that note.

Credits for finding and reporting this vulnerability to:
- The Raw (https://github.com/therawdev)
- Vishal (https://github.com/shukla304)

Signed-off-by: Erik Michelson <github@erik.michelson.eu>
---
 backend/src/api/private/notes/notes.controller.ts |  7 +++++--
 backend/src/api/public/notes/notes.controller.ts  |  7 +++++--
 backend/src/revisions/revisions.service.spec.ts   | 14 ++++++++------
 backend/src/revisions/revisions.service.ts        |  4 +++-
 4 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/backend/src/api/private/notes/notes.controller.ts b/backend/src/api/private/notes/notes.controller.ts
index 3a60105c9..fedd7032d 100644
--- a/backend/src/api/private/notes/notes.controller.ts
+++ b/backend/src/api/private/notes/notes.controller.ts
@@ -176,8 +176,11 @@ export class NotesController {
   @OpenApi(200, 404)
   @RequirePermission(PermissionLevel.READ)
   @UseInterceptors(GetNoteIdInterceptor)
-  async getNoteRevision(@Param('revisionUuid') revisionUuid: string): Promise<RevisionDto> {
-    return await this.revisionsService.getRevisionDto(revisionUuid);
+  async getNoteRevision(
+    @Param('revisionUuid') revisionUuid: string,
+    @RequestNoteId() noteId: number,
+  ): Promise<RevisionDto> {
+    return await this.revisionsService.getRevisionDto(revisionUuid, noteId);
   }
 
   @Put(':noteAlias/metadata/permissions/users/:username')
diff --git a/backend/src/api/public/notes/notes.controller.ts b/backend/src/api/public/notes/notes.controller.ts
index a84294bb4..5087bce3a 100644
--- a/backend/src/api/public/notes/notes.controller.ts
+++ b/backend/src/api/public/notes/notes.controller.ts
@@ -362,8 +362,11 @@ export class NotesController {
     },
     404,
   )
-  async getNoteRevision(@Param('revisionUuid') revisionUuid: string): Promise<RevisionDto> {
-    return await this.revisionsService.getRevisionDto(revisionUuid);
+  async getNoteRevision(
+    @Param('revisionUuid') revisionUuid: string,
+    @RequestNoteId() noteId: number,
+  ): Promise<RevisionDto> {
+    return await this.revisionsService.getRevisionDto(revisionUuid, noteId);
   }
 
   @UseInterceptors(GetNoteIdInterceptor)
diff --git a/backend/src/revisions/revisions.service.spec.ts b/backend/src/revisions/revisions.service.spec.ts
index 6b804e943..ab3e54478 100644
--- a/backend/src/revisions/revisions.service.spec.ts
+++ b/backend/src/revisions/revisions.service.spec.ts
@@ -219,11 +219,13 @@ describe('RevisionsService', () => {
           FieldNameRevision.patch,
         ],
         TableRevision,
-        FieldNameRevision.uuid,
+        [FieldNameRevision.uuid, FieldNameRevision.noteId],
         [],
       );
-      await expect(service.getRevisionDto(mockRevisionUuid1)).rejects.toThrow(NotInDBError);
-      expectBindings(tracker, 'select', [[mockRevisionUuid1]], true);
+      await expect(service.getRevisionDto(mockRevisionUuid1, mockNoteId)).rejects.toThrow(
+        NotInDBError,
+      );
+      expectBindings(tracker, 'select', [[mockRevisionUuid1, mockNoteId]], true);
     });
 
     it('correctly returns the fetched revision', async () => {
@@ -238,7 +240,7 @@ describe('RevisionsService', () => {
           FieldNameRevision.patch,
         ],
         TableRevision,
-        FieldNameRevision.uuid,
+        [FieldNameRevision.uuid, FieldNameRevision.noteId],
         [
           {
             [FieldNameRevision.uuid]: mockRevisionUuid1,
@@ -253,7 +255,7 @@ describe('RevisionsService', () => {
           },
         ],
       );
-      const result = await service.getRevisionDto(mockRevisionUuid1);
+      const result = await service.getRevisionDto(mockRevisionUuid1, mockNoteId);
       expect(result).toStrictEqual({
         uuid: mockRevisionUuid1,
         content: mockContent1,
@@ -263,7 +265,7 @@ describe('RevisionsService', () => {
         description: mockDescription,
         patch: mockPatch,
       });
-      expectBindings(tracker, 'select', [[mockRevisionUuid1]], true);
+      expectBindings(tracker, 'select', [[mockRevisionUuid1, mockNoteId]], true);
     });
   });
 
diff --git a/backend/src/revisions/revisions.service.ts b/backend/src/revisions/revisions.service.ts
index 3838e4225..f23f55045 100644
--- a/backend/src/revisions/revisions.service.ts
+++ b/backend/src/revisions/revisions.service.ts
@@ -203,10 +203,11 @@ export class RevisionsService {
    * Get a revision by its UUID
    *
    * @param revisionUuid The UUID of the revision to get
+   * @param noteId The id of the note this revision belongs to
    * @returns The revision DTO
    * @throws NotInDBError if the revision with the given UUID does not exist
    */
-  async getRevisionDto(revisionUuid: string): Promise<RevisionDto> {
+  async getRevisionDto(revisionUuid: string, noteId: number): Promise<RevisionDto> {
     const revision = await this.knex(TableRevision)
       .select(
         FieldNameRevision.uuid,
@@ -217,6 +218,7 @@ export class RevisionsService {
         FieldNameRevision.patch,
       )
       .where(FieldNameRevision.uuid, revisionUuid)
+      .andWhere(FieldNameRevision.noteId, noteId)
       .first();
     if (revision === undefined) {
       throw new NotInDBError(