mirror of
https://github.com/wekan/wekan.git
synced 2026-11-07 11:02:33 -05:00
The activity hook delivered webhooks through Meteor.call('outgoingWebhooks'),
and that method trusted a call without this.connection as the server's own.
Meteor gives a server-side Meteor.call the userId AND connection of the method
it runs in (ddp-server applyAsync), and a card moved from a browser runs in
that browser's method. So the server's own act-moveCard delivery arrived as a
client forging one: it was recorded as ssrf.webhook-forge (high) with the
mover's userId, and blockOnSecurityEvent set loginDisabled.
- deliverActivityWebhook() in server/notifications/outgoing.js is the server's
delivery: a function, not a method, so no client reaches it and no inherited
context changes it. It reads the integration again (enabled) and names the
activity's actor. server/models/activities.js calls it through safeDeliver.
- The outgoingWebhooks method treats every call as a client's: only the
card-opened notification, rebuilt from the card, as before for clients.
- Server deliveries no longer need the actor to be a member of the hook's
board, which also skipped a global webhook ('_global' is no board) and an
activity with no method behind it (REST, timers).
- securityLog.record() now passes the catalog key and blocksAccount to the
block decision. The stored row carries neither, so every 'high' blocked row
disabled whoever it named: a hook refused for its own internal address
disabled the mover too, and ssrf.redirect/ssrf.attachment's
blocksAccount: false never applied. ssrf.webhook is now blocksAccount:
false - the person whose move sent it did not write the hook's URL.
tests/hookBleedServerDelivery.test.cjs: the move delivered with nothing
logged, in an inherited context too; a forged call still refused, logged and
blocking; the card-opened notification; failing and internal hooks blocking
nobody, through record() itself; and a whole-tree scan that no server code
calls a this.connection-gated method from inside a client's method without
clearing the invocation. tests/playwright/specs/hook-bleed.e2e.js moves a
card through the Move dialog with a webhook enabled.
Thanks to do4me and xet7 !
Fixes #6782.
128 lines
6.5 KiB
JavaScript
128 lines
6.5 KiB
JavaScript
// ============================================================================
|
|
// WeKan security event logger (design: docs/Security/Remediation/WeKan.md)
|
|
// ----------------------------------------------------------------------------
|
|
// Single choke point every runtime security guard funnels through when it
|
|
// BLOCKS, SANITIZES or REMEDIATES a request (SSRF rejection, XSS scheme drop,
|
|
// forged-XFF ignore, export-authz denial, invite rate-limit, attachment/avatar
|
|
// upload rejection or sanitization, …).
|
|
//
|
|
// Records one document into the EXISTING WeKan database via a normal Meteor
|
|
// JavaScript query (models/eventLog.js, stream:'security'). No new files/DBs, so
|
|
// it works the same on FerretDB and MongoDB. BEST-EFFORT and NEVER throws into the
|
|
// caller: the insert is fire-and-forget and any error is swallowed. Server-only.
|
|
// ============================================================================
|
|
|
|
import EventLog from '/models/eventLog';
|
|
const { categoryFor } = require('/models/lib/securityCategories');
|
|
const { sanitizeDetail } = require('/models/lib/securityLogFormat');
|
|
import { foldEventFireAndForget } from '/server/lib/eventLogFold';
|
|
import { blockAccountForSecurityEvent } from '/server/lib/blockOnSecurityEvent';
|
|
import { currentReportRequest } from '/server/lib/requestReportContext';
|
|
const { resolveClientKey } = require('/server/lib/loginAttemptThrottle');
|
|
const { locationFromHeaders } = require('/models/lib/geoHeaders');
|
|
|
|
// ONE ROW PER PROBLEM, not per event (models/lib/eventLogSummary.js).
|
|
//
|
|
// A guard on a path an attacker controls fires as fast as they can send, so a
|
|
// row per event grows the database with the attack and buries the one line that
|
|
// mattered under ten thousand identical ones. The row carries `count` and the
|
|
// window `firstAt` … `at` instead, and the actor fields describe the most recent
|
|
// occurrence.
|
|
//
|
|
// Still fire-and-forget and still never throws into the caller: recording that a
|
|
// guard fired must never be able to stop it firing.
|
|
// ONE ROW PER PROBLEM, not per event: server/lib/eventLogFold.js does the
|
|
// folding, shared with the speed and test loggers. A guard on a path an attacker
|
|
// controls fires as fast as they can send, so a row per event grows the database
|
|
// with the attack and buries the one line that mattered under ten thousand
|
|
// identical ones. The row carries `count`, the window `firstAt` … `at`, and a
|
|
// per-actor tally read out as `username1 25, 100.100.100.100 30`.
|
|
function insert(doc, decision = {}) {
|
|
foldEventFireAndForget(doc, 'securityLog');
|
|
// A logged-in account that attempted a vulnerability loses that account - see
|
|
// server/lib/blockOnSecurityEvent.js for why the ACCOUNT and never the
|
|
// address. Fire-and-forget like the fold: the guard already refused the
|
|
// operation, and the consequence must not be able to break the refusal.
|
|
try {
|
|
// #6782: with the catalog key and its blocksAccount, which the stored row
|
|
// does not carry. Without them every 'high' blocked row disabled whoever it
|
|
// named - a webhook refused for its own internal URL disabled the user
|
|
// whose card move happened to send it, though ssrf.redirect,
|
|
// ssrf.attachment and ssrf.webhook all say blocksAccount: false.
|
|
const p = blockAccountForSecurityEvent({ ...doc, key: decision.key, blocksAccount: decision.blocksAccount });
|
|
if (p && typeof p.catch === 'function') p.catch(() => {});
|
|
} catch (e) { /* blocking must never break the guard */ }
|
|
}
|
|
|
|
// Record one security event. Never throws. Pass a catalog `key` (see
|
|
// models/lib/securityCategories) and/or explicit fields; explicit wins.
|
|
export function record(evt = {}) {
|
|
try {
|
|
const base = evt.key ? categoryFor(evt.key) : {};
|
|
const m = { ...base, ...evt };
|
|
const req = m.req || m.request || currentReportRequest();
|
|
if (req) {
|
|
if (!m.userId && req.userId) m.userId = req.userId;
|
|
if (!m.ip) {
|
|
m.ip = resolveClientKey({
|
|
headers: req.headers,
|
|
socketAddress: (req.socket && req.socket.remoteAddress)
|
|
|| (req.connection && req.connection.remoteAddress),
|
|
forwardedCount: process.env.HTTP_FORWARDED_COUNT,
|
|
});
|
|
}
|
|
if (!m.location) m.location = locationFromHeaders(req.headers);
|
|
}
|
|
if ((!m.userId || !m.ip) && typeof DDP !== 'undefined'
|
|
&& DDP._CurrentMethodInvocation) {
|
|
const invocation = DDP._CurrentMethodInvocation.get();
|
|
if (invocation) {
|
|
if (!m.userId && invocation.userId) m.userId = invocation.userId;
|
|
if (!m.ip && invocation.connection) {
|
|
m.ip = resolveClientKey({
|
|
headers: invocation.connection.httpHeaders,
|
|
socketAddress: invocation.connection.clientAddress,
|
|
forwardedCount: process.env.HTTP_FORWARDED_COUNT,
|
|
});
|
|
}
|
|
if (!m.location && invocation.connection) {
|
|
m.location = locationFromHeaders(invocation.connection.httpHeaders);
|
|
}
|
|
}
|
|
}
|
|
const doc = {
|
|
stream: 'security',
|
|
at: new Date(),
|
|
severity: m.severity || 'info',
|
|
category: m.category || 'unknown',
|
|
bleed: m.bleed || 'Generic',
|
|
action: m.action || 'detected',
|
|
source: m.source || '',
|
|
cwe: m.cwe || '',
|
|
detail: sanitizeDetail(m.detail),
|
|
};
|
|
if (m.userId || m.userid) doc.userId = String(m.userId || m.userid);
|
|
// The account an event is ABOUT, when that is not the actor - a lockout
|
|
// names the account being guessed. Never blocked; see blockOnSecurityEvent.
|
|
if (m.targetUserId) doc.targetUserId = String(m.targetUserId).slice(0, 64);
|
|
// WHO and FROM WHERE (docs/Security/Remediation/WeKan.md §12). Both are
|
|
// optional: a guard that fires on an unauthenticated request has no
|
|
// username, and one that fires outside a request context has no address.
|
|
// Truncated here as well as sanitized, because both are attacker-influenced
|
|
// strings - a username can be chosen at registration and an XFF header is
|
|
// whatever was sent.
|
|
if (m.username) doc.username = String(m.username).slice(0, 100);
|
|
if (m.ip) doc.ip = String(m.ip).slice(0, 64);
|
|
if (m.location) doc.location = m.location;
|
|
// How many attempts this row stands for; 1 unless a canary is flushing a
|
|
// window's summary.
|
|
const count = Number(m.count);
|
|
doc.count = Number.isFinite(count) && count > 0 ? Math.floor(count) : 1;
|
|
insert(doc, { key: m.key, blocksAccount: m.blocksAccount });
|
|
} catch (e) {
|
|
if (process.env.DEBUG === 'true') console.warn('securityLog.record failed:', e && e.message);
|
|
}
|
|
}
|
|
|
|
export default { record };
|