Skip to content

🐛 PUT-1656: stop an embedded fs: in a path from escalating to full FS access - #3666

Merged
Salazareo merged 1 commit into
mainfrom
juancastro/put-1656-fs-permission-rewriter-embedded-fs-escalates-a-scoped-grant
Aug 29, 2026
Merged

🐛 PUT-1656: stop an embedded fs: in a path from escalating to full FS access#3666
Salazareo merged 1 commit into
mainfrom
juancastro/put-1656-fs-permission-rewriter-embedded-fs-escalates-a-scoped-grant

Conversation

@jfcastro92

Copy link
Copy Markdown
Collaborator

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:/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 `/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.

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.
@jfcastro92 jfcastro92 changed the title fix: stop an embedded fs: in a path from escalating to full FS access 🐛 PUT-1656: stop an embedded fs: in a path from escalating to full FS access Aug 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 93.72%
🟰 ±0%
24860 / 26524
🔵 Statements 91.98%
🟰 ±0%
26850 / 29190
🔵 Functions 89.95%
🟰 ±0%
4350 / 4836
🔵 Branches 80.62%
🟰 ±0%
18121 / 22476
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/backend/services/fs/FSService.ts 93.77%
⬆️ +0.01%
85.61%
⬆️ +0.06%
98.85%
🟰 ±0%
94.42%
⬆️ +0.01%
110, 197, 229, 237, 266, 273, 304, 334, 341, 454-456, 466-468, 548, 576, 584-586, 615, 666-670, 773, 860, 877, 890, 921, 928, 937-940, 945-947, 1018-1020, 1250-1253, 1265, 1274, 1290-1293, 1310, 1448-1450, 1628-1632, 1648-1650, 1708-1710, 1731, 1765-1767, 1920-1922, 1959-1961, 2057-2059, 2100-2102, 2123-2130, 2149-2151, 2157-2159, 2206-2208, 2214-2216, 2229-2232, 2239-2241, 2299-2303, 2320-2324, 2340-2344, 2425-2429, 2443-2456, 2605-2609, 2623-2636, 2669-2672, 2731-2733, 2756-2758, 2824, 3193-3197, 3225-3229, 3344, 3502, 3746, 3815, 4086, 4193, 4221-4223
Generated in workflow #868 for commit af655fe by the Vitest Coverage Report Action

@jfcastro92
jfcastro92 requested a review from Salazareo August 29, 2026 02:41
@Salazareo
Salazareo merged commit cdfd38b into main Aug 29, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants