From 0dda149c7f528a7e98a34a8459b7afb23c90a05b Mon Sep 17 00:00:00 2001 From: Erik Michelson Date: Thu, 23 Jul 2026 22:17:16 +0200 Subject: [PATCH] fix(permissions): ignore unknown permission values The permission checks did not check for unknown permission values submitted to the server. This allowed bypassing permission checks under certain circumstances. However, this is not as critical as it might sound at first, since checks guard that only the owner can modify note permissions and the owner already has always full permission to their note. Signed-off-by: Erik Michelson --- lib/realtime.js | 37 ++++++++++++++++++++++++++++++------ public/docs/release-notes.md | 4 ++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/lib/realtime.js b/lib/realtime.js index 35b86266..3903eb6f 100644 --- a/lib/realtime.js +++ b/lib/realtime.js @@ -29,6 +29,8 @@ const realtime = { maintenance: true } +const allowedPermissionValues = ['freely', 'editable', 'limited', 'locked', 'protected', 'private'] + function onAuthorizeSuccess (data, accept) { accept() } @@ -363,13 +365,25 @@ function connectNextSocket () { } function checkViewPermission (req, note) { - if (note.permission === 'private') { - if (req.user && req.user.logged_in && req.user.id === note.owner) { return true } else { return false } - } else if (note.permission === 'limited' || note.permission === 'protected') { - if (req.user && req.user.logged_in) { return true } else { return false } - } else { + const permission = note.permission + + // Private notes should only be readable to their owner + if (permission === 'private') { + return req.user && req.user.logged_in && req.user.id === note.owner + } + + // Limited and protected notes should be readable by all logged-in users + if (permission === 'limited' || permission === 'protected') { + return req.user && req.user.logged_in + } + + // Freely, Editable and Locked notes should be readable by everyone + if (permission === 'freely' || permission === 'editable' || permission === 'locked') { return true } + + // Invalid permission + return false } let isConnectionBusy = false @@ -636,6 +650,10 @@ function ifMayEdit (socket, callback) { // only owner can change if (!note.owner || note.owner !== socket.request.user.id) { mayEdit = false } break + default: + // invalid permission + mayEdit = false + break } // if user may edit and this is a text operation if (socket.origin === 'operation' && mayEdit) { @@ -778,7 +796,14 @@ function connection (socket) { const note = notes[noteId] // Only owner can change permission if (note.owner && note.owner === socket.request.user.id) { - if (permission === 'freely' && !config.allowAnonymous && !config.allowAnonymousEdits) return + // If config disallows freely permission, do not set it + if (permission === 'freely' && !config.allowAnonymous && !config.allowAnonymousEdits) { + return + } + // Ignore invalid permission values + if (!allowedPermissionValues.includes(permission)) { + return + } note.permission = permission models.Note.update({ permission diff --git a/public/docs/release-notes.md b/public/docs/release-notes.md index a06bc417..5679c7f5 100644 --- a/public/docs/release-notes.md +++ b/public/docs/release-notes.md @@ -2,6 +2,10 @@ ## 1.11.1 2026-07-24 +### Security fixes + +- [GHSA-93w7-49m2-cqwg](https://github.com/hedgedoc/hedgedoc/security/advisories/GHSA-93w7-49m2-cqwg) reports possible corruption of permission values due to missing validation. This should not impact permission checks, except when the note owner intentionally set their note permission to something invalid. + ### Enhancements - Added external link warning setting (`externalLinkWarning` in config.json or `CMD_EXTERNAL_LINK_WARNING`) to disable the external link warning page entirely