Skip to content

Commit 642dec2

Browse files
melvincarvalhomelvincarvalho
committed
caldav/carddav: cap member walks to bound DoS amplification
Audit finding: free-busy-query, calendar/addressbook-query, multiget and PROPFIND Depth:1 walked every container member (or every client-supplied <href>) with a full-body buffer each, uncapped — unlike sparql/rss/ search/backup. Add config.maxResources (default 10000, matching backup) truncating the member/href list before the walk: normal collections untouched, an oversized one gets a truncated-but-valid 207 instead of unbounded loopback fetches. Regression tests mount a second instance at a low cap and assert truncation.
1 parent 6ae408d commit 642dec2

4 files changed

Lines changed: 162 additions & 7 deletions

File tree

‎caldav/plugin.js‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ const DAV_ALLOW = 'OPTIONS, GET, HEAD, PUT, DELETE, PROPFIND, REPORT, MKCOL, MKC
5454
const XML_TYPE = 'application/xml; charset=utf-8';
5555
const ICAL_TYPE = 'text/calendar';
5656

57+
// Cap the number of members a single member-walk / multiget touches, to bound
58+
// the loopback fan-out: each member is a full-body loopback GET, so an uncapped
59+
// walk over an attacker-grown collection (or a multiget with an arbitrarily
60+
// long <href> list) drives N fetches + N full-body buffers per request. A
61+
// collection larger than the cap gets a truncated but valid multistatus rather
62+
// than unbounded work — the same maxResources cap backup/, sparql/, rss/ and
63+
// search/ apply to their loopback walks. Config-overridable via
64+
// config.maxResources.
65+
const DEFAULT_MAX_RESOURCES = 10000;
66+
5767
const xmlEscape = (s) => String(s).replace(/[<>&'"]/g, (c) => (
5868
{ '<': '&lt;', '>': '&gt;', '&': '&amp;', "'": '&apos;', '"': '&quot;' }[c]
5969
));
@@ -270,6 +280,10 @@ export async function activate(api) {
270280
const calName = api.config.calendar || 'calendar';
271281
// Optional Apple calendar colour, echoed as IC:calendar-color.
272282
const calColor = api.config.color || null;
283+
// Upper bound on members touched by any single member-walk / multiget.
284+
const maxResources = api.config.maxResources ?? DEFAULT_MAX_RESOURCES;
285+
/** Truncate a member/href list to the cap, bounding loopback fan-out. */
286+
const capMembers = (arr) => (arr.length > maxResources ? arr.slice(0, maxResources) : arr);
273287

274288
// ---------------------------------------------------------------- auth
275289
function bridgeAuth(request) {
@@ -485,7 +499,7 @@ export async function activate(api) {
485499
const { props, missing } = collectionProps(path, want, pod);
486500
responses.push(response(prefix + path, props, missing));
487501
if (depth === 1 && listing) {
488-
for (const childPath of childPaths(listing, path)) {
502+
for (const childPath of capMembers(childPaths(listing, path))) {
489503
if (childPath === path) continue;
490504
if (childPath.endsWith('/')) {
491505
const { props: cp, missing: cm } = collectionProps(childPath, want, pod);
@@ -537,7 +551,7 @@ export async function activate(api) {
537551
if (res.status === 401) return unauthorized(reply);
538552
if (!res.ok) return reply.code(res.status).send();
539553
const periods = [];
540-
for (const childPath of childPaths(listing, coll)) {
554+
for (const childPath of capMembers(childPaths(listing, coll))) {
541555
if (childPath.endsWith('/') || !childPath.endsWith('.ics')) continue;
542556
const got = await getResource(childPath, auth); // unreadable → skipped, not busy
543557
if (!got.body) continue;
@@ -584,7 +598,7 @@ export async function activate(api) {
584598
if (multiget) {
585599
// multiget: the client lists exactly the hrefs it wants back.
586600
const hrefs = [...xml.matchAll(/<[A-Za-z]*:?href>\s*([^<]+?)\s*<\/[A-Za-z]*:?href>/gi)].map((m) => m[1]);
587-
for (const href of hrefs) {
601+
for (const href of capMembers(hrefs)) {
588602
const hp = hrefToHostPath(href.trim());
589603
if (!hp) {
590604
responses.push(`<D:response><D:href>${xmlEscape(href.trim())}</D:href>`
@@ -610,7 +624,7 @@ export async function activate(api) {
610624
const { res, listing } = await fetchListing(coll, auth);
611625
if (res.status === 401) return unauthorized(reply);
612626
if (!res.ok) return reply.code(res.status).send();
613-
for (const childPath of childPaths(listing, coll)) {
627+
for (const childPath of capMembers(childPaths(listing, coll))) {
614628
if (childPath.endsWith('/') || !childPath.endsWith('.ics')) continue;
615629
const got = await getResource(childPath, auth);
616630
if (!got.body) continue;

‎caldav/test.js‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,18 @@ describe('caldav plugin', () => {
6262
port,
6363
idp: true,
6464
plugins: [{
65+
id: 'caldav',
6566
module: path.join(__dirname, 'plugin.js'),
6667
prefix: '/caldav',
6768
config: { baseUrl: base },
69+
}, {
70+
// A second mount of the same bridge with a deliberately tiny
71+
// maxResources, to exercise the DoS cap (member-walk / multiget
72+
// truncation) without seeding 10 000 events.
73+
id: 'caldavcap',
74+
module: path.join(__dirname, 'plugin.js'),
75+
prefix: '/caldavcap',
76+
config: { baseUrl: base, maxResources: 3 },
6877
}],
6978
});
7079
const res = await fetch(`${base}/.pods`, {
@@ -389,4 +398,57 @@ describe('caldav plugin', () => {
389398
const bad = await fetch(cd('/freebusy/cal?start=banana&end=20260725T000000Z'), { headers: bearer() });
390399
assert.strictEqual(bad.status, 400);
391400
});
401+
402+
// ------------------------------------------------------------- DoS cap
403+
// Every member-walk / multiget is capped at config.maxResources so a single
404+
// request over an attacker-grown collection can't drive unbounded loopback
405+
// fetches + full-body buffers. The /caldavcap mount was booted with a tiny
406+
// cap (3); the default /caldav mount proves normal collections are untouched.
407+
it('maxResources cap truncates the member-walk and multiget instead of hanging', async () => {
408+
const names = ['cap1.ics', 'cap2.ics', 'cap3.ics', 'cap4.ics', 'cap5.ics'];
409+
for (const n of names) {
410+
const put = await fetch(cd(`/cal/capcal/${n}`), {
411+
method: 'PUT',
412+
headers: { ...bearer(), 'content-type': 'text/calendar' },
413+
body: vevent(n, 'DTSTART:20260720T090000Z', 'DTEND:20260720T100000Z', `SUMMARY:${n}`),
414+
});
415+
assert.ok([200, 201, 204].includes(put.status), `seed ${n}: ${put.status}`);
416+
}
417+
418+
const icsHrefs = (xml) => (xml.match(/\.ics<\/D:href>/g) || []).length;
419+
420+
// Uncapped mount: all 5 members listed (normal behaviour preserved).
421+
const full = await fetch(cd('/cal/capcal/'), {
422+
method: 'PROPFIND',
423+
headers: { ...bearer(), depth: '1', 'content-type': 'application/xml' },
424+
body: '<?xml version="1.0"?><D:propfind xmlns:D="DAV:"><D:prop><D:getetag/></D:prop></D:propfind>',
425+
});
426+
assert.strictEqual(full.status, 207);
427+
assert.strictEqual(icsHrefs(await full.text()), 5, 'uncapped mount should list every member');
428+
429+
// Low-cap mount: the PROPFIND Depth 1 member-walk is truncated to the cap.
430+
const capped = await fetch(`${base}/caldavcap/cal/capcal/`, {
431+
method: 'PROPFIND',
432+
headers: { ...bearer(), depth: '1', 'content-type': 'application/xml' },
433+
body: '<?xml version="1.0"?><D:propfind xmlns:D="DAV:"><D:prop><D:getetag/></D:prop></D:propfind>',
434+
});
435+
assert.strictEqual(capped.status, 207, 'capped PROPFIND still returns a valid multistatus');
436+
const cappedCount = icsHrefs(await capped.text());
437+
assert.ok(cappedCount <= 3, `member-walk not capped: listed ${cappedCount}`);
438+
assert.ok(cappedCount < 5, `cap did not truncate: ${cappedCount}`);
439+
440+
// multiget: an over-long <href> list is capped the same way.
441+
const hrefs = names.map((n) => `<D:href>/caldavcap/cal/capcal/${n}</D:href>`).join('');
442+
const mg = await fetch(`${base}/caldavcap/cal/capcal/`, {
443+
method: 'REPORT',
444+
headers: { ...bearer(), 'content-type': 'application/xml' },
445+
body: '<?xml version="1.0"?>'
446+
+ '<CAL:calendar-multiget xmlns:D="DAV:" xmlns:CAL="urn:ietf:params:xml:ns:caldav">'
447+
+ '<D:prop><D:getetag/><CAL:calendar-data/></D:prop>'
448+
+ `${hrefs}</CAL:calendar-multiget>`,
449+
});
450+
assert.strictEqual(mg.status, 207);
451+
const mgCount = ((await mg.text()).match(/<CAL:calendar-data>/g) || []).length;
452+
assert.ok(mgCount <= 3, `multiget not capped: returned ${mgCount} bodies`);
453+
});
392454
});

‎carddav/plugin.js‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@ const DAV_ALLOW = 'OPTIONS, GET, HEAD, PUT, DELETE, PROPFIND, REPORT, MKCOL';
4242
const XML_TYPE = 'application/xml; charset=utf-8';
4343
const VCARD_TYPE = 'text/vcard';
4444

45+
// Cap the number of members a single member-walk / multiget touches, to bound
46+
// the loopback fan-out: each member is a full-body loopback GET, so an uncapped
47+
// walk over an attacker-grown collection (or a multiget with an arbitrarily
48+
// long <href> list) drives N fetches + N full-body buffers per request. A
49+
// collection larger than the cap gets a truncated but valid multistatus rather
50+
// than unbounded work — the same maxResources cap backup/, sparql/, rss/ and
51+
// search/ apply to their loopback walks. Config-overridable via
52+
// config.maxResources.
53+
const DEFAULT_MAX_RESOURCES = 10000;
54+
4555
const xmlEscape = (s) => String(s).replace(/[<>&'"]/g, (c) => (
4656
{ '<': '&lt;', '>': '&gt;', '&': '&amp;', "'": '&apos;', '"': '&quot;' }[c]
4757
));
@@ -98,6 +108,10 @@ export async function activate(api) {
98108
// The container name that carries the addressbook resourcetype. A collection
99109
// whose final segment is this is advertised as a CARD:addressbook.
100110
const bookName = api.config.addressbook || 'contacts';
111+
// Upper bound on members touched by any single member-walk / multiget.
112+
const maxResources = api.config.maxResources ?? DEFAULT_MAX_RESOURCES;
113+
/** Truncate a member/href list to the cap, bounding loopback fan-out. */
114+
const capMembers = (arr) => (arr.length > maxResources ? arr.slice(0, maxResources) : arr);
101115

102116
// ---------------------------------------------------------------- auth
103117
function bridgeAuth(request) {
@@ -298,7 +312,7 @@ export async function activate(api) {
298312
const { props, missing } = collectionProps(path, want, pod);
299313
responses.push(response(prefix + path, props, missing));
300314
if (depth === 1 && listing) {
301-
for (const childPath of childPaths(listing, path)) {
315+
for (const childPath of capMembers(childPaths(listing, path))) {
302316
if (childPath === path) continue;
303317
if (childPath.endsWith('/')) {
304318
const { props: cp, missing: cm } = collectionProps(childPath, want, pod);
@@ -351,7 +365,7 @@ export async function activate(api) {
351365
if (multiget) {
352366
// multiget: the client lists exactly the hrefs it wants back.
353367
const hrefs = [...xml.matchAll(/<[A-Za-z]*:?href>\s*([^<]+?)\s*<\/[A-Za-z]*:?href>/gi)].map((m) => m[1]);
354-
for (const href of hrefs) {
368+
for (const href of capMembers(hrefs)) {
355369
const hp = hrefToHostPath(href.trim());
356370
if (!hp) {
357371
responses.push(`<D:response><D:href>${xmlEscape(href.trim())}</D:href>`
@@ -377,7 +391,7 @@ export async function activate(api) {
377391
const { res, listing } = await fetchListing(coll, auth);
378392
if (res.status === 401) return unauthorized(reply);
379393
if (!res.ok) return reply.code(res.status).send();
380-
for (const childPath of childPaths(listing, coll)) {
394+
for (const childPath of capMembers(childPaths(listing, coll))) {
381395
if (childPath.endsWith('/') || !childPath.endsWith('.vcf')) continue;
382396
const got = await getResource(childPath, auth);
383397
if (!got.body) continue;

‎carddav/test.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,18 @@ describe('carddav plugin', () => {
5757
port,
5858
idp: true,
5959
plugins: [{
60+
id: 'carddav',
6061
module: path.join(__dirname, 'plugin.js'),
6162
prefix: '/carddav',
6263
config: { baseUrl: base },
64+
}, {
65+
// A second mount of the same bridge with a deliberately tiny
66+
// maxResources, to exercise the DoS cap (member-walk / multiget
67+
// truncation) without seeding 10 000 contacts.
68+
id: 'carddavcap',
69+
module: path.join(__dirname, 'plugin.js'),
70+
prefix: '/carddavcap',
71+
config: { baseUrl: base, maxResources: 3 },
6372
}],
6473
});
6574
const res = await fetch(`${base}/.pods`, {
@@ -217,4 +226,60 @@ describe('carddav plugin', () => {
217226
});
218227
assert.strictEqual(res.status, 207);
219228
});
229+
230+
// ------------------------------------------------------------- DoS cap
231+
// Every member-walk / multiget is capped at config.maxResources so a single
232+
// request over an attacker-grown collection can't drive unbounded loopback
233+
// fetches + full-body buffers. The /carddavcap mount was booted with a tiny
234+
// cap (3); the default /carddav mount proves normal collections are untouched.
235+
it('maxResources cap truncates the member-walk and multiget instead of hanging', async () => {
236+
const vcard = (uid) => [
237+
'BEGIN:VCARD', 'VERSION:3.0', `UID:urn:uuid:${uid}`, `FN:${uid}`, `N:${uid};;;;`, 'END:VCARD', '',
238+
].join('\r\n');
239+
const names = ['cap1.vcf', 'cap2.vcf', 'cap3.vcf', 'cap4.vcf', 'cap5.vcf'];
240+
for (const n of names) {
241+
const put = await fetch(cd(`/cara/capbook/${n}`), {
242+
method: 'PUT',
243+
headers: { ...bearer(), 'content-type': 'text/vcard' },
244+
body: vcard(n),
245+
});
246+
assert.ok([200, 201, 204].includes(put.status), `seed ${n}: ${put.status}`);
247+
}
248+
249+
const vcfHrefs = (xml) => (xml.match(/\.vcf<\/D:href>/g) || []).length;
250+
251+
// Uncapped mount: all 5 members listed (normal behaviour preserved).
252+
const full = await fetch(cd('/cara/capbook/'), {
253+
method: 'PROPFIND',
254+
headers: { ...bearer(), depth: '1', 'content-type': 'application/xml' },
255+
body: '<?xml version="1.0"?><D:propfind xmlns:D="DAV:"><D:prop><D:getetag/></D:prop></D:propfind>',
256+
});
257+
assert.strictEqual(full.status, 207);
258+
assert.strictEqual(vcfHrefs(await full.text()), 5, 'uncapped mount should list every member');
259+
260+
// Low-cap mount: the PROPFIND Depth 1 member-walk is truncated to the cap.
261+
const capped = await fetch(`${base}/carddavcap/cara/capbook/`, {
262+
method: 'PROPFIND',
263+
headers: { ...bearer(), depth: '1', 'content-type': 'application/xml' },
264+
body: '<?xml version="1.0"?><D:propfind xmlns:D="DAV:"><D:prop><D:getetag/></D:prop></D:propfind>',
265+
});
266+
assert.strictEqual(capped.status, 207, 'capped PROPFIND still returns a valid multistatus');
267+
const cappedCount = vcfHrefs(await capped.text());
268+
assert.ok(cappedCount <= 3, `member-walk not capped: listed ${cappedCount}`);
269+
assert.ok(cappedCount < 5, `cap did not truncate: ${cappedCount}`);
270+
271+
// multiget: an over-long <href> list is capped the same way.
272+
const hrefs = names.map((n) => `<D:href>/carddavcap/cara/capbook/${n}</D:href>`).join('');
273+
const mg = await fetch(`${base}/carddavcap/cara/capbook/`, {
274+
method: 'REPORT',
275+
headers: { ...bearer(), 'content-type': 'application/xml' },
276+
body: '<?xml version="1.0"?>'
277+
+ '<CARD:addressbook-multiget xmlns:D="DAV:" xmlns:CARD="urn:ietf:params:xml:ns:carddav">'
278+
+ '<D:prop><D:getetag/><CARD:address-data/></D:prop>'
279+
+ `${hrefs}</CARD:addressbook-multiget>`,
280+
});
281+
assert.strictEqual(mg.status, 207);
282+
const mgCount = ((await mg.text()).match(/<CARD:address-data>/g) || []).length;
283+
assert.ok(mgCount <= 3, `multiget not capped: returned ${mgCount} bodies`);
284+
});
220285
});

0 commit comments

Comments
 (0)