Skip to content

Commit e9f2509

Browse files
committed
docs: Harden extras path-traversal protection (raw+decoded checks)
1 parent f935536 commit e9f2509

1 file changed

Lines changed: 65 additions & 13 deletions

File tree

docs/superpowers/specs/2026-05-22-extras-wildcard-design.md

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -93,23 +93,68 @@ Validation at boot:
9393

9494
## Path-traversal protection
9595

96-
Before any matching, the router validates the decoded request path
97-
(`HttpExchange.getRequestURI().getPath()`, which is already
98-
percent-decoded by the JDK) against these rules:
96+
Before any matching, the router validates both the raw URI and the decoded
97+
request path. The raw check catches encoded traversal tricks; the decoded
98+
check catches literal traversal segments. Either failure throws
99+
`BadRequestException` and the `ExceptionFilter` renders problem+json 400.
99100

100-
- No segment equals `.` or `..`.
101-
- No empty segment (no `//` in the path).
102-
- No NUL byte (U+0000) anywhere in the path.
101+
### Raw URI rules (`HttpExchange.getRequestURI().getRawPath()`)
103102

104-
A violation throws `BadRequestException` and the `ExceptionFilter` renders
105-
the standard problem+json 400. The check runs once per request inside the
106-
`ExtrasRouter`, before exact-or-wildcard dispatch, so even handlers that
107-
chose to read the URI cannot see a traversal-laden path.
103+
Reject the request if the raw path contains any of the following — these
104+
have no legitimate use in our routes and are common encoding tricks:
105+
106+
- `%2e` or `%2E` (encoded `.`) — also defeats double-encoding (`%252e…`,
107+
which decodes once to `%2e…` and would otherwise sneak past the
108+
decoded-segment check).
109+
- `%2f` or `%2F` (encoded `/`).
110+
- `%5c` or `%5C` (encoded backslash).
111+
- `%00` (encoded NUL — truncation attacks).
112+
- A literal backslash `\` (some libraries treat it as a separator).
113+
- Any control char in U+0000–U+001F or U+007F.
114+
115+
### Decoded path rules (`HttpExchange.getRequestURI().getPath()`)
116+
117+
After the JDK's single-pass percent-decoding, split on `/` and reject if:
118+
119+
- Any segment equals `.` or `..`.
120+
- Any segment is empty (`//` anywhere in the path).
121+
- The decoded path contains NUL (U+0000) or any other control char
122+
(U+0001–U+001F, U+007F).
123+
- Decoding raises `URISyntaxException` or `IllegalArgumentException`
124+
(malformed / overlong encoding) — caught and rethrown as
125+
`BadRequestException`.
126+
127+
### Order
128+
129+
1. Apply raw-URI rules to `getRawPath()`.
130+
2. Decode via `getPath()`; if decoding fails, 400.
131+
3. Apply decoded-path rules.
132+
4. Only then run exact-or-wildcard dispatch.
133+
134+
This runs once per request inside `ExtrasRouter`, before any handler is
135+
consulted, so even handlers that choose to read the URI cannot see a
136+
traversal-laden path.
137+
138+
### Handler responsibility
139+
140+
The router stops traversal at the URI layer; it cannot police what a
141+
handler does with the matched-but-not-exposed path. Any future handler
142+
that maps a request portion to a filesystem location MUST also:
143+
144+
- Resolve the target against a fixed base directory.
145+
- Canonicalise via `Path.toRealPath()` and assert
146+
`resolved.startsWith(baseReal)`.
147+
- Refuse symlinks that escape the base.
148+
149+
This document does not add such a handler, but the rule is recorded here
150+
so it survives the next time someone adds one.
151+
152+
### Out of scope (deliberate)
108153

109154
The same validation does NOT run inside the basePath spec context — spec
110155
paths are matched against an explicit template set, so a `..` segment
111156
simply fails the exact/template match and yields a normal 404. Adding the
112-
400 check there is out of scope (mentioned for clarity, not implemented).
157+
400 check there is mentioned for clarity but not implemented.
113158

114159
## Error handling
115160

@@ -138,8 +183,15 @@ Tests added under `src/test/java/com/retailsvc/http/`:
138183
- `/schemas/**/openapi.yaml` serves the spec at various depths
139184
- exact extras still work (regression for `ExactUrlMatchingIT` scenarios)
140185
- basePath spec routes still take precedence over extras
141-
- path-traversal: `/files/../etc/passwd`, `/files/%2e%2e/etc/passwd`,
142-
`/files/.`, `/files//x` all return 400
186+
- path-traversal — all return 400:
187+
- decoded: `/files/../etc/passwd`, `/files/./x`, `/files//x`
188+
- single-encoded: `/files/%2e%2e/etc/passwd`, `/files/%2E/x`
189+
- double-encoded: `/files/%252e%252e/etc/passwd`
190+
- encoded slash: `/files/%2fetc/passwd`
191+
- backslash: `/files/..\etc\passwd` (literal and `%5c`)
192+
- NUL truncation: `/files/x%00.txt`
193+
- control char: `/files/x%0a/y`
194+
- malformed encoding: `/files/%zz`
143195

144196
## Out of scope
145197

0 commit comments

Comments
 (0)