Skip to content

Commit b9b38ed

Browse files
fix(wac): require Control on protected resource for POST-created .acl/.meta sidecars (JavaScriptSolidServer#580)
* fix(wac): require Control on protected resource for POST-created .acl/.meta sidecars A POST whose Slug resolves to an `.acl`/`.meta` sidecar is currently authorized only against the *container* (the request path), because the dedicated ACL Control guard in `auth/middleware.js` (`authorizeAclAccess`) keys on `urlPath.endsWith('.acl')` — which never matches a container POST. The sidecar filename is only produced *inside* `handlePost` via `generateUniqueFilename`, after authorization has run. Impact: an agent holding only `acl:Append` on a container (e.g. a public-append inbox/upload directory created by `generateInboxAcl`) can `POST` with `Slug: victim.acl` and write a resource ACL that grants itself `acl:Control`/`acl:Read` on a sibling — privilege escalation to full control of a resource it had no access to. The slug validator permits `.`, so `victim.acl` passes. Fix: in `handlePost`, when the resolved child filename ends in `.acl`/`.meta`, require `acl:Control` on the protected resource (the sidecar path minus the suffix) before writing, mirroring `authorizeAclAccess`. Owners (who hold Control) are unaffected; Append-only agents get 403. Found during a cross-implementation audit against the solid-pod-rs Rust port, which shared the same gap and is fixed in lockstep. Reproduction and a proposed regression test are in the PR description; the full integration harness could not be exercised in the contributor's environment (missing optional `@simplewebauthn/server` dep used by the passkey path at server bootstrap), so CI validation is requested. Co-Authored-By: jjohare <github@thedreamlab.uk> * test(wac): regression for POST-created .acl/.meta sidecar injection Cover the privilege-escalation path fixed in this PR: an append-only agent (public inbox) POSTing Slug: victim.acl / victim.meta must get 403, a normal non-sidecar POST still gets 201, and the owner (Control) can still POST an .acl sidecar. The deny test fails against the unpatched handler and passes with the Control guard. * fix(wac): use buildResourceUrl for sidecar Control check; clarify .meta rationale Address review feedback on the POST .acl/.meta sidecar guard: - Build the protected-resource URL with buildResourceUrl() (the same helper authorize()/authorizeAclAccess() use) instead of a hand-rolled request.hostname string, so the Control decision is evaluated against the identical origin (host+port, subdomain-normalized) as the rest of WAC. - Reword the code and test comments: only .acl is consulted for WAC; .meta is gated as defense-in-depth (protected Solid sidecar), not because it governs permissions. * fix(wac): don't debit ledger in the secondary sidecar Control check The POST .acl/.meta guard in handlePost runs a second checkAccess() on a request the global authorize() hook already evaluated (and possibly billed). Since checkAccess() debits the web ledger for a matching positive-cost PaymentCondition, a payment-gated Control grant could be charged inside the guard — a double debit, and a silent one (the guard ignores paid/ paymentRequired, so no X-Cost/X-Balance headers and 403 instead of 402). Add a noDebit option to checkAccess()/checkAuthorizations(): when set, a positive-cost paid grant is treated as not-satisfied (returns paymentRequired) rather than debited. Owners hold unconditioned Control and are unaffected; the authoritative debit stays in the primary authorize() path. The sidecar guard passes noDebit: true. Adds test/wac.test.js coverage asserting the primary check debits while the noDebit check leaves the balance unchanged. * docs(wac): document full checkAccess() return shape The @returns for checkAccess() listed only {allowed, wacAllow}, but the function also returns paymentRequired/paid/balance/currency, which callers (and the noDebit test) rely on. Document the full shape. --------- Co-authored-by: Melvin Carvalho <melvincarvalho@gmail.com>
1 parent 0cb00a3 commit b9b38ed

4 files changed

Lines changed: 197 additions & 5 deletions

File tree

src/handlers/container.js

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import { initializeQuota, checkQuota, updateQuotaUsage } from '../storage/quota.
33
import { getAllHeaders } from '../ldp/headers.js';
44
import { isContainer, getEffectiveUrlPath, getPodName } from '../utils/url.js';
55
import { generateProfile, generatePreferences, generateTypeIndex, serialize } from '../webid/profile.js';
6-
import { generateOwnerAcl, generatePrivateAcl, generateInboxAcl, generatePublicFolderAcl, serializeAcl, relativizeOwnerWebId } from '../wac/parser.js';
6+
import { generateOwnerAcl, generatePrivateAcl, generateInboxAcl, generatePublicFolderAcl, serializeAcl, relativizeOwnerWebId, AccessMode } from '../wac/parser.js';
7+
import { checkAccess } from '../wac/checker.js';
8+
import { buildResourceUrl } from '../auth/middleware.js';
79
import { provisionOwnerKey, assertProvisionKeysCompatible } from '../keys/provision.js';
810
import { createToken } from '../auth/token.js';
911
import { canAcceptInput, toJsonLd, RDF_TYPES } from '../rdf/conneg.js';
@@ -88,6 +90,43 @@ export async function handlePost(request, reply) {
8890
const newStoragePath = storagePath + filename + (isCreatingContainer ? '/' : '');
8991
const resourceUrl = `${request.protocol}://${request.hostname}${newUrlPath}`;
9092

93+
// Security: a Slug that resolves to an `.acl` sidecar governs ANOTHER
94+
// resource's permissions — the WAC checker searches for `*.acl`, so an
95+
// `.acl` written here becomes the authorization policy for its sibling.
96+
// The authorize() preHandler only checked Append/Write on the *container*
97+
// (the request path), and its dedicated `.acl` Control guard
98+
// (authorizeAclAccess) never fires here because the request path is the
99+
// container, not the resolved sidecar. Without this an agent with mere
100+
// Append rights on a container could POST `Slug: victim.acl` and self-grant
101+
// Control on a sibling resource — privilege escalation. `.meta` is not
102+
// consulted for WAC, but it is a protected Solid sidecar dotfile, so we gate
103+
// it the same way (defense in depth) rather than let it be minted by Append.
104+
// Mirror authorizeAclAccess: require acl:Control on the protected resource
105+
// before minting a sidecar via POST. Build the resource URL with the same
106+
// buildResourceUrl() the auth middleware uses so this Control decision is
107+
// evaluated against the identical origin (host+port, subdomain-normalized).
108+
// noDebit: this is a secondary WAC check on a request the authorize() hook
109+
// already evaluated (and possibly billed) — pass noDebit so a payment-gated
110+
// Control grant can't be charged here (no double debit, no silent charge).
111+
if (!isCreatingContainer && /\.(acl|meta)$/.test(filename)) {
112+
const protectedUrlPath = newUrlPath.replace(/\.(acl|meta)$/, '');
113+
const protectedStoragePath = newStoragePath.replace(/\.(acl|meta)$/, '');
114+
const { allowed } = await checkAccess({
115+
resourceUrl: buildResourceUrl(request, protectedUrlPath),
116+
resourcePath: protectedStoragePath,
117+
isContainer: protectedUrlPath.endsWith('/'),
118+
agentWebId: request.webId,
119+
requiredMode: AccessMode.CONTROL,
120+
noDebit: true
121+
});
122+
if (!allowed) {
123+
return reply.code(403).send({
124+
error: 'Forbidden',
125+
message: 'Creating an ACL/meta sidecar via POST requires Control on the protected resource'
126+
});
127+
}
128+
}
129+
91130
let success;
92131
if (isCreatingContainer) {
93132
success = await storage.createContainer(newStoragePath);

src/wac/checker.js

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,32 @@ import { readLedger, getBalance, debit } from '../webledger.js';
1616
* @param {boolean} options.isContainer - Whether resource is a container
1717
* @param {string|null} options.agentWebId - WebID of the agent (null for unauthenticated)
1818
* @param {string} options.requiredMode - Required access mode (from AccessMode)
19-
* @returns {Promise<{allowed: boolean, wacAllow: string}>}
19+
* @param {boolean} [options.noDebit=false] - When true, evaluate a
20+
* PaymentCondition without charging the ledger. A positive-cost paid grant
21+
* is treated as not-satisfied (returns paymentRequired) rather than debited.
22+
* Used by secondary/guard checks (e.g. the POST sidecar Control gate in
23+
* handlePost) so a single request cannot debit twice or charge silently;
24+
* the authoritative debit stays in the primary authorize() hook.
25+
* @returns {Promise<{
26+
* allowed: boolean,
27+
* wacAllow: string,
28+
* paymentRequired?: object|null,
29+
* paid?: number,
30+
* balance?: number,
31+
* currency?: string
32+
* }>}
33+
* `paymentRequired` carries the unmet PaymentCondition (present when a paid
34+
* grant is denied, including every `noDebit` denial). `paid`/`balance`/
35+
* `currency` are set only when a debit actually occurred. The no-ACL deny
36+
* path returns just `{allowed, wacAllow}`.
2037
*/
2138
export async function checkAccess({
2239
resourceUrl,
2340
resourcePath,
2441
isContainer,
2542
agentWebId,
26-
requiredMode
43+
requiredMode,
44+
noDebit = false
2745
}) {
2846
// Find applicable ACL
2947
const aclResult = await findApplicableAcl(resourceUrl, resourcePath, isContainer);
@@ -43,7 +61,8 @@ export async function checkAccess({
4361
resourceUrl, // Use actual resource URL, not the ACL container URL
4462
agentWebId,
4563
requiredMode,
46-
isDefault
64+
isDefault,
65+
noDebit
4766
);
4867

4968
// Calculate WAC-Allow header
@@ -129,7 +148,7 @@ function getParentPath(path) {
129148
// Supported condition types
130149
const SUPPORTED_CONDITIONS = ['PaymentCondition', 'https://webacl.org/ns#PaymentCondition'];
131150

132-
async function checkAuthorizations(authorizations, targetUrl, agentWebId, requiredMode, isDefault) {
151+
async function checkAuthorizations(authorizations, targetUrl, agentWebId, requiredMode, isDefault, noDebit = false) {
133152
for (const auth of authorizations) {
134153
// For default ACLs, check if auth has default rules and matches target
135154
// For direct ACLs, check if accessTo matches target
@@ -183,6 +202,11 @@ async function checkAuthorizations(authorizations, targetUrl, agentWebId, requir
183202
// Paid access: check balance and deduct
184203
const balance = getBalance(ledger, agentWebId, currency);
185204
if (cost > 0 && balance >= cost) {
205+
// Guard checks must not charge: a paid grant is left unsatisfied
206+
// here so billing happens once, in the primary authorize() path.
207+
if (noDebit) {
208+
return { allowed: false, paymentRequired: paymentCondition };
209+
}
186210
const result = debit(ledger, agentWebId, cost, currency);
187211
const { writeLedger } = await import('../webledger.js');
188212
await writeLedger(ledger);

test/auth.test.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,60 @@ describe('Authentication', () => {
212212
const res3 = await request('/authuser1/authenticated-only/test.txt', { auth: 'authuser2' });
213213
assertStatus(res3, 200);
214214
});
215+
216+
it('should deny POST-created .acl/.meta sidecars without Control on the protected resource', async () => {
217+
// Regression for the POST .acl sidecar injection: an agent holding only
218+
// acl:Append on a container (here, the public-append inbox) must not be
219+
// able to plant an .acl sidecar, which the WAC checker would then treat
220+
// as the authorization policy for the sibling resource. .meta is not a
221+
// WAC input, but is gated the same way as a protected Solid sidecar.
222+
await createTestPod('sidecarvictim');
223+
224+
const aclBody = JSON.stringify({
225+
'@context': { acl: 'http://www.w3.org/ns/auth/acl#' },
226+
'@graph': []
227+
});
228+
229+
// Append-only (unauthenticated public append) agent tries to plant victim.acl
230+
const attackAcl = await request('/sidecarvictim/inbox/', {
231+
method: 'POST',
232+
headers: { 'Content-Type': 'application/json', 'Slug': 'victim.acl' },
233+
body: aclBody
234+
});
235+
assertStatus(attackAcl, 403);
236+
237+
// The same trick with a .meta sidecar must also be blocked
238+
const attackMeta = await request('/sidecarvictim/inbox/', {
239+
method: 'POST',
240+
headers: { 'Content-Type': 'application/json', 'Slug': 'victim.meta' },
241+
body: aclBody
242+
});
243+
assertStatus(attackMeta, 403);
244+
245+
// A normal (non-sidecar) POST to the public inbox still works
246+
const legit = await request('/sidecarvictim/inbox/', {
247+
method: 'POST',
248+
headers: { 'Content-Type': 'application/json', 'Slug': 'note' },
249+
body: JSON.stringify({ type: 'note' })
250+
});
251+
assertStatus(legit, 201);
252+
});
253+
254+
it('should allow the owner (Control) to POST an .acl sidecar', async () => {
255+
// Owners hold acl:Control, so the sidecar guard must not block them.
256+
await createTestPod('sidecarowner');
257+
258+
const res = await request('/sidecarowner/', {
259+
method: 'POST',
260+
headers: { 'Content-Type': 'application/json', 'Slug': 'owned.acl' },
261+
body: JSON.stringify({
262+
'@context': { acl: 'http://www.w3.org/ns/auth/acl#' },
263+
'@graph': []
264+
}),
265+
auth: 'sidecarowner'
266+
});
267+
assertStatus(res, 201);
268+
});
215269
});
216270

217271
describe('WAC-Allow Header', () => {

test/wac.test.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import {
2525
relativizeOwnerWebId
2626
} from '../src/wac/parser.js';
2727
import { checkAccess, getRequiredMode } from '../src/wac/checker.js';
28+
import * as storage from '../src/storage/filesystem.js';
29+
import { createLedger, setBalance, getBalance, LEDGER_PATH } from '../src/webledger.js';
2830

2931
describe('WAC Parser', () => {
3032
describe('parseAcl', () => {
@@ -713,3 +715,76 @@ describe('WAC Conditions', () => {
713715
});
714716
});
715717
});
718+
719+
describe('WAC PaymentCondition noDebit (secondary/guard checks must not charge)', () => {
720+
let baseUrl;
721+
const AGENT = 'https://payer.example/profile/card#me';
722+
const COST = 5;
723+
const RESOURCE_PATH = '/paygate/resource';
724+
725+
before(async () => {
726+
const result = await startTestServer();
727+
baseUrl = result.baseUrl;
728+
});
729+
730+
after(async () => {
731+
await stopTestServer();
732+
});
733+
734+
// Seed a ledger balance for AGENT and an ACL granting AGENT Control on the
735+
// protected resource, gated behind a positive-cost PaymentCondition.
736+
async function seed(balance) {
737+
const ledger = createLedger();
738+
setBalance(ledger, AGENT, balance, 'sat');
739+
await storage.write(LEDGER_PATH, Buffer.from(JSON.stringify(ledger)));
740+
741+
const resourceUrl = `${baseUrl}${RESOURCE_PATH}`;
742+
const acl = {
743+
'@context': { acl: 'http://www.w3.org/ns/auth/acl#' },
744+
'@graph': [{
745+
'@id': '#paid',
746+
'@type': 'acl:Authorization',
747+
'acl:agent': { '@id': AGENT },
748+
'acl:accessTo': { '@id': resourceUrl },
749+
'acl:mode': [{ '@id': 'acl:Control' }],
750+
'acl:condition': { '@type': 'PaymentCondition', amount: String(COST), currency: 'sat' }
751+
}]
752+
};
753+
await storage.write(`${RESOURCE_PATH}.acl`, Buffer.from(JSON.stringify(acl)));
754+
return resourceUrl;
755+
}
756+
757+
async function balanceNow() {
758+
const raw = await storage.read(LEDGER_PATH);
759+
return getBalance(JSON.parse(raw.toString()), AGENT, 'sat');
760+
}
761+
762+
it('debits the ledger on a normal (primary) Control check', async () => {
763+
const resourceUrl = await seed(100);
764+
const res = await checkAccess({
765+
resourceUrl,
766+
resourcePath: RESOURCE_PATH,
767+
isContainer: false,
768+
agentWebId: AGENT,
769+
requiredMode: AccessMode.CONTROL
770+
});
771+
assert.strictEqual(res.allowed, true);
772+
assert.strictEqual(res.paid, COST);
773+
assert.strictEqual(await balanceNow(), 100 - COST, 'primary check should debit');
774+
});
775+
776+
it('does NOT debit when noDebit is set (guard/secondary check)', async () => {
777+
const resourceUrl = await seed(100);
778+
const res = await checkAccess({
779+
resourceUrl,
780+
resourcePath: RESOURCE_PATH,
781+
isContainer: false,
782+
agentWebId: AGENT,
783+
requiredMode: AccessMode.CONTROL,
784+
noDebit: true
785+
});
786+
assert.strictEqual(res.allowed, false, 'paid grant is not satisfied without charging');
787+
assert.ok(res.paymentRequired, 'should surface paymentRequired instead of debiting');
788+
assert.strictEqual(await balanceNow(), 100, 'balance must be unchanged');
789+
});
790+
});

0 commit comments

Comments
 (0)