mirror of
https://github.com/wekan/wekan.git
synced 2026-11-07 11:02:33 -05:00
Lauri Ojansivu
3151f8a815
The reply-by-email token signed only the card, so every recipient of a card's notification held the same Reply-To address, and the webhook took the comment author from the reply's From address - a field the sender controls. Anyone holding one card's reply address could comment on it as any user (GHSA-mc7c-cv99-64h7, CWE-346). Validation also found that server/routes/inboundEmail.js was never imported by server/imports.js, so released versions did not register /api/inbound-email: the documented feature did not work and the flaw could not be reached. The endpoint is registered now, together with the fix. The reply address is now reply+<cardId>.<userId>.<expiry>.<mac>: an HMAC-SHA256 over the card, the recipient and the last valid day (INBOUND_EMAIL_REPLY_DAYS, default 30). The author is that recipient; the From address must be one of the recipient's own addresses; the recipient must be enabled and still allowed to comment on the card, including the assigned-only restriction. INBOUND_EMAIL_WEBHOOK_SECRET, when set, must be presented by the provider (X-WeKan-Inbound-Secret header or ?secret=) and is compared in constant time. The old card-only token is refused. A forged token, a spoofed sender and a wrong provider secret are recorded as ReplyBleed in Admin Panel -> Problems; expired and pre-fix tokens and lost access are refused quietly, since real users replying to old mail reach them. Tests: token round trip, per-recipient addresses, tampering, expiry and legacy tokens; the author decision reproducing the advisory's attack; a tree-wide negative that no code picks an author from a From address; and a full-app HTTP test against /api/inbound-email. Thanks to alex131125 and xet7 !
49 lines
2.4 KiB
JavaScript
49 lines
2.4 KiB
JavaScript
'use strict';
|
|
|
|
// Who wrote an inbound reply (#2414), decided without a database so the tests
|
|
// can drive it: tests/inboundEmailUserMatch.test.cjs.
|
|
//
|
|
// ReplyBleed (GHSA-mc7c-cv99-64h7): this module used to pick the author by
|
|
// searching EVERY user for the reply's From address - a field the sender
|
|
// controls - so anyone holding a card's reply address could comment as anyone.
|
|
// The author is now the recipient named in the signed reply token, and the From
|
|
// address only has to agree with it.
|
|
|
|
const { memberCan } = require('../../models/lib/boardRoleCapabilities');
|
|
const { isAssignedOnlyMember } = require('../../models/lib/boardCardScope');
|
|
|
|
function normalizeEmail(address) {
|
|
if (typeof address !== 'string') return '';
|
|
// A From header can be "Name <addr@x>" as well as a bare address.
|
|
const angleMatch = address.match(/<([^>]+)>/);
|
|
const raw = angleMatch ? angleMatch[1] : address;
|
|
return raw.trim().toLowerCase();
|
|
}
|
|
|
|
// Pure: is `fromAddress` one of THIS user's own addresses?
|
|
function senderIsUser(fromAddress, user) {
|
|
const email = normalizeEmail(fromAddress);
|
|
if (!email || !user || !Array.isArray(user.emails)) return false;
|
|
return user.emails.some(e => e && typeof e.address === 'string' && e.address.toLowerCase() === email);
|
|
}
|
|
|
|
// Pure: may the token's recipient `user` comment on `card` of `board` with a
|
|
// reply from `fromAddress`? Returns { ok: true } or { ok: false, code, error,
|
|
// log } - `log` is true only for what nothing but an attempt produces: a From
|
|
// address that is not the recipient's. A disabled user or a member who has
|
|
// lost access since the email was sent is a real person, so it is not logged.
|
|
function replyAuthorDecision({ user, fromAddress, card, board }) {
|
|
if (!user || user.loginDisabled) return { ok: false, code: 403, error: 'Unrecognized sender', log: false };
|
|
if (!senderIsUser(fromAddress, user)) {
|
|
return { ok: false, code: 403, error: 'Unrecognized sender', log: true,
|
|
detail: 'sender address is not the reply address recipient' };
|
|
}
|
|
if (!card || !board || board._id !== card.boardId || !memberCan(board.members, user._id, 'comment') ||
|
|
(isAssignedOnlyMember(board, user._id) && !(card.assignees || []).includes(user._id))) {
|
|
return { ok: false, code: 403, error: 'Not allowed to comment on this card', log: false };
|
|
}
|
|
return { ok: true };
|
|
}
|
|
|
|
module.exports = { normalizeEmail, senderIsUser, replyAuthorDecision };
|