More security issues addressed.

This commit is contained in:
Scott Duensing 2026-07-01 20:05:17 -05:00
parent ed953d398c
commit 80866305c2
2 changed files with 28 additions and 6 deletions

View file

@ -97,12 +97,20 @@ const handler = async (req, res, next) => {
// cap. A presence filter like (uid=*) is NOT equality (no .value) and
// falls through to the capped scan.
const f = req.filter;
const attr = f && f.attribute ? String(f.attribute).toLowerCase() : null;
// A leaf equality on uid/mail (same structural test as the existing
// single-equality fast path). Reused to classify OR children below.
// A leaf filter (equality) exposes .clauses/.filters as an EMPTY array
// (the same array instance in @ldapjs/filter), which is TRUTHY -- so the
// old `!node.clauses` guard was ALWAYS false, making both fast paths dead
// code and forcing every search into the capped full scan (a user past
// the cap could then never be resolved). Test emptiness by length.
const noChildren = (node) => {
const kids = (node && (node.clauses || node.filters)) || [];
return kids.length === 0;
};
// A leaf equality on uid/mail (same structural test as the single-
// equality fast path). Reused to classify OR children below.
const isUidMailEquality = (node) => {
const a = node && node.attribute ? String(node.attribute).toLowerCase() : null;
return !!node && (a === "uid" || a === "mail") && node.value !== undefined && node.value !== null && !node.filters && !node.clauses;
return !!node && (a === "uid" || a === "mail") && node.value !== undefined && node.value !== null && noChildren(node);
};
// OR children expose siblings via .filters (ldapjs) or .clauses (alias).
const orChildren = (node) => {
@ -117,7 +125,7 @@ const handler = async (req, res, next) => {
return Array.isArray(kids) ? kids : null;
};
let users;
if (f && (attr === "uid" || attr === "mail") && f.value !== undefined && f.value !== null && !f.filters && !f.clauses) {
if (isUidMailEquality(f)) {
const one = await User.findOne({ email: String(f.value) });
users = one ? [one] : [];
} else if (orChildren(f) && orChildren(f).length > 0 && orChildren(f).every(isUidMailEquality)) {

View file

@ -22,6 +22,13 @@ const { issuerForReq } = require("./discovery");
const COOKIE_KEY_INFO = "saltcorn-idp:oidc-cookie-keys:v1";
const COOKIE_KEY_BYTES = 32;
// Hard cap on distinct cached Providers. The issuer is derived from the request
// Host header when base_url is unset (see issuerForReq), so without a bound an
// anonymous attacker spraying distinct Host values could grow this Map -- each
// entry a full Provider -- until the worker OOMs. A real deployment has one issuer
// per tenant, far below this; past the cap we evict the oldest (insertion order).
const MAX_PROVIDERS = 256;
let providerClassPromise = null;
const providersByIssuer = new Map();
@ -116,6 +123,13 @@ const getProviderEntry = async (req) => {
const provider = await buildProvider(issuer);
entry = { provider: provider, handler: provider.callback(), fingerprint: fingerprint };
providersByIssuer.set(issuer, entry);
// Bound the cache: evict the oldest entry once we exceed the cap. A brand
// new issuer is inserted last, so the oldest key is always a different,
// older entry -- never the one just built.
if (providersByIssuer.size > MAX_PROVIDERS) {
const oldest = providersByIssuer.keys().next().value;
providersByIssuer.delete(oldest);
}
}
return entry;
};