fix: stop an embedded fs: in a path from escalating to full FS access (#3666)

The fs-path-to-uid permission rewriter split permission strings on the raw
\`fs:\` substring instead of parsing on component boundaries. A path can
itself contain \`fs:\` — a home dir named \`…fs\`, or \`fs\` in the mode
position — and the raw split mistook that for the mode delimiter.

Crafting \`fs:/<victim>fs:junk:read\` made the split do two things at once:
it dropped the \`junk:read\` mode, and it consumed the trailing \`fs\`, turning
the harmless-looking nonexistent path shown in the consent dialog into the
victim's real home path. The rewriter then stored a bare \`fs:<home_uuid>\`,
which subsumes every mode via parent-permission matching — full
read+write+delete from a request the user saw as \`junk:read\`.

Parse with PermissionUtil.split() in both matches() and rewrite(), matching
every other permission parser, and locate the \`fs\` component by position
(after an optional \`manage\` prefix). The path component is now exactly what
sits between \`fs\` and the next unescaped colon, and all trailing components
are preserved as the mode — so an embedded \`fs:\` is data, never a delimiter.
The crafted input now addresses the nonexistent \`/<victim>fs\` and 404s.

Regression tests pin both halves: the exact PoC must 404, and an \`fs:\` in
the mode position must survive the rewrite instead of collapsing to a bare
permission. Both fail on the old rewriter and pass on the fix.
This commit is contained in:
Juan Fernando Castro
2026-08-29 01:41:40 -07:00
committed by GitHub
parent 693e86d763
commit cdfd38bd4b
2 changed files with 30 additions and 7 deletions
+20
View File
@@ -3367,6 +3367,26 @@ describe('FSService permission rules', () => {
expect(error.legacyCode).toBe('subject_does_not_exist');
});
// The old raw `split('fs:')` dropped the mode and stripped the trailing `fs`, resolving this to a bare `fs:<home_uuid>` that subsumes every mode.
it('does not let an embedded fs: escalate a scoped grant to a bare one', async () => {
const error = await caught(() =>
server.services.permission.rewritePermission(
`fs:${user.home}fs:junk:read`,
),
);
expect(error.statusCode).toBe(404);
expect(error.legacyCode).toBe('subject_does_not_exist');
});
it('keeps the mode when a later component contains fs:', async () => {
// `fs` in the mode position is data, not a delimiter: the path resolves and the mode is preserved, never collapsed to bare.
await expect(
server.services.permission.rewritePermission(
`fs:${file.path}:fs:read`,
),
).resolves.toBe(`fs:${file.uuid}:fs:read`);
});
it('leaves uuid-addressed and non-fs permissions untouched', async () => {
await expect(
server.services.permission.rewritePermission(
+10 -7
View File
@@ -182,14 +182,18 @@ export class FSService extends PuterService {
!permission.startsWith(`${MANAGE_PERM_PREFIX}:fs:`)
)
return false;
const [, specifier] = permission.split('fs:');
// Parse on component boundaries: a path can contain `fs:` (e.g. a home dir named `…fs`), which a raw split mistakes for the mode delimiter.
const parts = PermissionUtil.split(permission);
const fsIndex = parts[0] === MANAGE_PERM_PREFIX ? 1 : 0;
const specifier = parts[fsIndex + 1];
return Boolean(specifier && specifier.startsWith('/'));
},
rewrite: async (permission: string): Promise<string> => {
const [manageOpt, pathPerm] = permission.split('fs:');
const parts = PermissionUtil.split(pathPerm);
const path = parts[0];
const rest = parts.slice(1);
const parts = PermissionUtil.split(permission);
const hasManage = parts[0] === MANAGE_PERM_PREFIX;
const fsIndex = hasManage ? 1 : 0;
const path = parts[fsIndex + 1];
const rest = parts.slice(fsIndex + 2);
if (!path) return permission;
const entry = await fsEntryStore.getEntryByPath(path);
if (!entry) {
@@ -197,9 +201,8 @@ export class FSService extends PuterService {
legacyCode: 'subject_does_not_exist',
});
}
const manage = manageOpt.replace(':', '');
const joined = PermissionUtil.join('fs', entry.uuid, ...rest);
return manage ? `${manage}:${joined}` : joined;
return hasManage ? `${MANAGE_PERM_PREFIX}:${joined}` : joined;
},
});