Close the SignupBleed DDP sibling: a client sign-up cannot claim admin or LDAP creation.

Accounts.onCreateUser trusts from: 'admin' (admin or REST creation, which
skips the registration checks) and ldap: true (directory account, allowed while
registration is disabled) as server-only signals, and Meteor's createUser DDP
method passes the client's options through unchanged. With "Disable
registration" on, anyone could still create an account by adding either.

The createUser method is wrapped so a client's options lose the server-only
keys; the server's own callers - the admin method, POST /api/users, the LDAP
package - call Accounts.createUserAsync directly and keep them. An attempt is
recorded as SignupBleed. The negative test fails if onCreateUser trusts another
option as an early-return signal without it being stripped from clients; a
browser test signs up both ways with registration disabled.

Thanks to xet7 !
This commit is contained in:
Lauri Ojansivu committed 2026-10-02 10:39:40 +03:00
1 parent 8fc9bd5a94
commit dd36db3641
5 files changed
+139

No files matched your search

+25
View File
@@ -0,0 +1,25 @@
'use strict';
// SignupBleed, DDP sibling (2026-10-02). Accounts.onCreateUser trusts two
// options as server-only signals: `from: 'admin'` (an admin or the REST API
// creating the account - it skips the registration checks) and `ldap: true`
// (the LDAP package creating an account for a directory user - allowed while
// registration is disabled). Meteor's `createUser` DDP method passes the
// CLIENT's options object to that hook unchanged, so with "Disable
// registration" on, anyone could still sign up by adding either option.
//
// The server's own callers (the admin method, POST /api/users, the LDAP
// package) call Accounts.createUserAsync directly and keep them. A client's
// createUser call never may. Pure: tests/signupBleedDdp.test.cjs.
const SERVER_ONLY_CREATION_OPTIONS = ['from', 'ldap'];
// The options a client may pass, and which server-only ones it tried to.
function clientCreationOptions(options) {
if (!options || typeof options !== 'object') return { options, refused: [] };
const refused = SERVER_ONLY_CREATION_OPTIONS.filter(key => Object.prototype.hasOwnProperty.call(options, key));
if (!refused.length) return { options, refused };
const clean = { ...options };
for (const key of refused) delete clean[key];
return { options: clean, refused };
}
module.exports = { SERVER_ONLY_CREATION_OPTIONS, clientCreationOptions };
+2
View File
@@ -155,6 +155,8 @@ import '/config/search-const';
import '/server/00checkStartup';
import '/server/accounts-common';
import '/server/accounts-lockout-config';
// SignupBleed: a client's createUser call cannot pass server-only options.
import '/server/lib/clientAccountCreationGuard';
// Fold the per-event eventlog rows an older WeKan wrote into problem summaries
// (server/lib/eventLogSummaryMigration.js). Idempotent and once.
import '/server/eventLogSummaryStartup';
+27
View File
@@ -0,0 +1,27 @@
// SignupBleed, DDP sibling: see models/lib/clientAccountCreation.js. Wraps the
// accounts-password `createUser` DDP method - the only way a client creates an
// account - so a client cannot pass the server-only creation options. Loaded
// from server/imports.js, after the accounts packages registered the method.
import { Meteor } from 'meteor/meteor';
const { clientCreationOptions } = require('/models/lib/clientAccountCreation');
const handlers = Meteor.server && Meteor.server.method_handlers;
const original = handlers && handlers.createUser;
if (typeof original === 'function' && !original.__wekanClientOptionsGuard) {
const guarded = async function createUser(options, ...rest) {
const { options: clean, refused } = clientCreationOptions(options);
if (refused.length) {
// Nobody signs up with these through the UI: an attempt to bypass
// "Disable registration". Not logged in yet, so no account to block.
try {
require('/server/lib/securityLog').record({
key: 'authz.register', action: 'blocked', source: 'ddp:createUser',
detail: `client sign-up passed server-only option(s): ${refused.join(', ')}`,
});
} catch (e) { /* logging must never break the guard */ }
}
return original.call(this, clean, ...rest);
};
guarded.__wekanClientOptionsGuard = true;
handlers.createUser = guarded;
}
@@ -0,0 +1,36 @@
'use strict';
// SignupBleed, DDP sibling (2026-10-02): with registration disabled, the
// client's createUser call still created an account when it carried
// from: 'admin' or ldap: true.
const { test, expect } = require('../fixtures');
const db = require('../helpers/db');
const { waitForMeteor } = require('../helpers/auth');
const BASE_URL = process.env.WEKAN_BASE_URL || 'http://localhost:3000';
test('with registration disabled, a client cannot sign up by claiming admin or LDAP creation', async ({ page }) => {
const setting = db.findOne('settings', {});
db.updateOne('settings', { _id: setting._id }, { $set: { disableRegistration: true } });
const names = [];
try {
await page.goto(`${BASE_URL}/sign-in`);
await waitForMeteor(page);
for (const extra of [{ from: 'admin' }, { ldap: true }]) {
const username = `signupbleed${Date.now()}${Object.keys(extra)[0]}`;
names.push(username);
const result = await page.evaluate(async ({ username, extra }) => {
const { Accounts } = Package['accounts-base'];
try {
await Accounts.createUserAsync({ username, email: `${username}@example.com`, password: 'Correct-Horse-1', ...extra });
return 'created';
} catch (error) { return error.error || error.message; }
}, { username, extra });
expect(result, JSON.stringify(extra)).not.toBe('created');
expect(db.findOne('users', { username })).toBeNull();
}
await expect.poll(() => db.findOne('eventlog', { bleed: 'SignupBleed', source: 'ddp:createUser' })?.count).toBeGreaterThanOrEqual(2);
} finally {
db.updateOne('settings', { _id: setting._id }, { $set: { disableRegistration: !!setting.disableRegistration } });
db.deleteMany('users', { username: { $in: names } });
}
});
+49
View File
@@ -0,0 +1,49 @@
'use strict';
// Guard: SignupBleed, DDP sibling (2026-10-02). With "Disable registration"
// on, Meteor's createUser DDP method still created an account for anyone who
// added `from: 'admin'` or `ldap: true` to the options - the hook that decides
// trusted them, and the method passes the client's options through unchanged.
// Run: node tests/signupBleedDdp.test.cjs
const { test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const ROOT = path.join(__dirname, '..');
const read = file => fs.readFileSync(path.join(ROOT, file), 'utf8');
const { clientCreationOptions, SERVER_ONLY_CREATION_OPTIONS } = require('../models/lib/clientAccountCreation');
test('the reported attack: a client sign-up loses the server-only options', () => {
for (const extra of [{ from: 'admin' }, { ldap: true }, { from: 'admin', ldap: true }]) {
const sent = { username: 'mallory', email: 'm@example.com', password: { digest: 'x', algorithm: 'sha-256' }, ...extra };
const { options, refused } = clientCreationOptions(sent);
assert.deepEqual(Object.keys(options).sort(), ['email', 'password', 'username']);
assert.deepEqual(refused.sort(), Object.keys(extra).sort());
assert.equal(sent.from, extra.from, 'the caller\'s object is not mutated');
}
// An ordinary sign-up, with an invitation code, passes unchanged (negative).
const ordinary = { username: 'u', email: 'u@example.com', password: 'x', profile: { invitationcode: 'abc' } };
assert.deepEqual(clientCreationOptions(ordinary), { options: ordinary, refused: [] });
assert.deepEqual(SERVER_ONLY_CREATION_OPTIONS, ['from', 'ldap']);
});
test('the createUser DDP method is wrapped, loaded, and records the attempt', () => {
const guard = read('server/lib/clientAccountCreationGuard.js');
assert.match(guard, /const original = handlers && handlers\.createUser;/);
assert.match(guard, /return original\.call\(this, clean, \.\.\.rest\);/);
assert.match(guard, /key: 'authz\.register', action: 'blocked', source: 'ddp:createUser'/);
assert.match(guard, /catch \(e\) \{ \/\* logging must never break the guard \*\/ \}/);
assert.match(read('server/imports.js'), /import '\/server\/lib\/clientAccountCreationGuard';/);
});
test('negative: every option onCreateUser trusts as a server-only signal is stripped from clients', () => {
const users = read('server/models/users.js');
const hook = users.slice(users.indexOf('Accounts.onCreateUser('), users.indexOf('\n});', users.indexOf('Accounts.onCreateUser(')));
// An option that returns early (skipping the registration checks) is a
// server-only signal; each must be in the stripped list.
const trusted = [...hook.matchAll(/options(?:\s*&&\s*options)?\.(\w+)(?:\s*===\s*'[^']*'|\s*===\s*true)?\)\s*\{[^}]*?return user;/g)].map(m => m[1]);
assert.ok(trusted.includes('from') && trusted.includes('ldap'), `found ${trusted}`);
for (const option of trusted) assert.ok(SERVER_ONLY_CREATION_OPTIONS.includes(option), `${option} is trusted but not stripped`);
});