diff --git a/src/utils/static.ts b/src/utils/static.ts index ca4b2d5ef..f7fbb85e4 100644 --- a/src/utils/static.ts +++ b/src/utils/static.ts @@ -84,14 +84,36 @@ export async function serveStatic( throw new HTTPError({ status: 405 }); } + // Resolve `.`/`..` traversal FIRST, then decode, so the on-disk id matches + // what `sirv`/`serve-static` serve (a filesystem-backed `getContents` no + // longer needs self-decoding logic — e.g. `/50%25.png` finds `50%.png`). + // // `event.url.pathname` is already decoded once by the event layer - // (`decodePathname`, a single `decodeURI` that preserves `%25`) — the same - // value the rest of h3 routes on. Decoding again here would peel a second - // `%25` level, so serveStatic would resolve/serve a different id than was - // dispatched (e.g. a double-encoded separator collapsing to `%2f`). - // `resolveDotSegments` decodes nested-encoded dot segments itself, so no - // extra decode is needed for traversal safety. - const originalId = resolveDotSegments(withoutTrailingSlash(event.url.pathname)); + // (`decodePathname`, a single `decodeURI` that preserves `%25`), and + // `resolveDotSegments` neutralizes every traversal escape (literal `../`, + // `..\`, and `%2e`-encoded dot segments at any `%25`-nesting depth). Only + // then do we `decodeURI` to peel one `%25` level (`%25` → `%`) for the + // lookup. This never reintroduces a separator: `decodeURI` preserves `%2f` + // (reserved), and a single-encoded `%5c` can't reach here — the event layer + // already decoded it to `\` and `resolveDotSegments` normalized that away, so + // only a double-encoded `%255c` survives and `decodeURI` collapses it to a + // literal `%5c`, not a raw `\`. + // + // The final decode is guarded: with `allowMalformedURL`, a raw malformed `%` + // (e.g. `/foo%`, `/%ZZ`) reaches here and `decodeURI` throws — fall back to + // the traversal-resolved (still-safe) value so `fallthrough`/404 handling is + // reached instead of a 500. A `%`-free path (the common case) skips the + // decode entirely, matching the fast-path guards at the event layer and in + // `resolveDotSegments`. + const resolvedId = resolveDotSegments(withoutTrailingSlash(event.url.pathname)); + let originalId = resolvedId; + if (resolvedId.includes("%")) { + try { + originalId = decodeURI(resolvedId); + } catch { + originalId = resolvedId; + } + } const acceptEncodings = parseAcceptEncoding( event.req.headers.get("accept-encoding") || "", diff --git a/test/static.test.ts b/test/static.test.ts index acff95744..9b8031ab9 100644 --- a/test/static.test.ts +++ b/test/static.test.ts @@ -1,5 +1,5 @@ -import { beforeEach, vi } from "vitest"; -import { serveStatic, type ServeStaticOptions } from "../src/index.ts"; +import { beforeEach, describe, it, expect, vi } from "vitest"; +import { H3, serveStatic, type ServeStaticOptions } from "../src/index.ts"; import { describeMatrix } from "./_setup.ts"; describeMatrix("serve static", (t, { it, expect }) => { @@ -163,32 +163,45 @@ describeMatrix("serve static", (t, { it, expect }) => { expect(text).not.toContain("%2E%2E"); }); - it("does not decode the pathname a second time", async () => { - // The event layer already applied the one canonical decode (preserving - // `%25`), so a double-encoded separator stays opaque and must reach the - // backend as the same id h3 routes on — not the twice-decoded `%2f`. + it("does not reintroduce a backslash separator when decoding the id", async () => { + // A double-encoded backslash traversal exercises the final `decodeURI`: + // `%255c` survives `resolveDotSegments` (a single `%5c` is decoded to `\` + // and normalized away one layer earlier), and the decode must collapse it + // to a *literal* `%5c` — never a raw `\`. So the whole thing stays one + // opaque segment: the `..` never becomes a bare path segment, and no new + // separator boundary is introduced. + const res = await t.fetch("/..%255c..%255cetc%255cpasswd"); + const text = await res.text(); + expect(text).not.toContain("\\"); // no raw backslash separator + expect(text).toContain("%5c"); // stayed literal, not decoded to `\` + // no bare `..` segment — boundaries include `\` so a decoded backslash + // separator would be caught too, not only a `/`. + expect(text).not.toMatch(/(^|[\\/])\.\.([\\/]|$)/); + }); + + it("decodes an encoded separator for the on-disk lookup", async () => { + // serveStatic resolves `.`/`..` traversal first, then decodes once more, so + // the id reaches the backend fully decoded — matching `sirv`/`serve-static` + // and letting a filesystem-backed `getContents` skip self-decoding. + // `/files/a%252fb` → id `a%2fb` (the encoded slash stays a literal `%2f` + // segment, so no traversal is reintroduced). const res = await t.fetch("/files/a%252fb"); expect(res.status).toEqual(200); - // A trailing encoding suffix (e.g. `.gz`) may be appended by the - // accept-encoding search, so match the distinguishing part loosely: - // the id keeps the single-decoded `%252f`, not the twice-decoded `%2f`. const text = await res.text(); - expect(text).toContain("a%252fb"); - expect(text).not.toContain("a%2fb"); + expect(text).toContain("a%2fb"); + expect(text).not.toContain("a%252fb"); }); - it("keeps a literal `%` in a filename encoded in the served id", async () => { - // Regression for the dropped second decode: a file whose name contains a - // literal `%` arrives as `%25` and the event layer preserves it, so the id - // reaches the backend as `/50%25.png` (the same value h3 routes on), not - // the old twice-decoded `/50%.png`. A filesystem-backed `getContents` must - // therefore decode `%25` itself. (See followup issue on aligning this with - // `sirv`/`serve-static`, which decode the id for the on-disk lookup.) + it("decodes a literal `%` in a filename for the on-disk lookup", async () => { + // A file whose name contains a literal `%` (`50%.png`) is requested as + // `/50%25.png`. The event layer preserves `%25`, then serveStatic decodes it + // once more so the id reaches the backend as `/50%.png` — the on-disk name — + // aligning with `sirv`/`serve-static` instead of forcing self-decoding. const res = await t.fetch("/50%25.png"); expect(res.status).toEqual(200); const text = await res.text(); - expect(text).toContain("/50%25.png"); - expect(text).not.toContain("/50%.png"); + expect(text).toContain("/50%.png"); + expect(text).not.toContain("/50%25.png"); }); it("allows legitimate paths with dots", async () => { @@ -374,3 +387,58 @@ describeMatrix("serve static MIME types", (t, { it, expect }) => { expect(res.headers.get("content-length")).toBe("999"); }); }); + +describe("serve static (malformed url)", () => { + it("falls through on a malformed `%` instead of throwing", async () => { + // With `allowMalformedURL`, a raw malformed `%` (e.g. `/foo%`) reaches + // serveStatic and the final `decodeURI` would throw a URIError — a 500 that + // bypasses `fallthrough`. The guarded decode falls back to the resolved id + // so `fallthrough` (and 404 when off) are reached as normal. + const options: ServeStaticOptions = { + getContents: () => undefined, + getMeta: () => undefined, + }; + + const fallthroughApp = new H3({ allowMalformedURL: true }) + .use((event) => serveStatic(event, { ...options, fallthrough: true })) + .use(() => "next"); + const res = await fallthroughApp.request("/foo%"); + expect(res.status).toBe(200); + expect(await res.text()).toBe("next"); + + const notFoundApp = new H3({ allowMalformedURL: true }).all("/**", (event) => + serveStatic(event, options), + ); + const res404 = await notFoundApp.request("/foo%"); + expect(res404.status).toBe(404); + }); + + it("never hands a raw backslash or bare `..` to the backend under allowMalformedURL", async () => { + // Defense-in-depth guard for the trickiest input we could construct: a + // malformed `%` (forcing the raw pathname) whose segment is popped by a + // `..`, leaving single-encoded `%5c` separators. If the on-disk decode + // (`%5c` → `\`) were not neutralized, the id would become + // `/..\..\windows\win.ini` — a traversal above the root on backslash-aware + // (e.g. Windows) filesystem backends. It is neutralized because the event + // layer's URL normalization collapses `\` → `/` and resolves `..` when the + // decoded pathname is assigned back, *before* serveStatic runs. This asserts + // that invariant end-to-end so a future event-layer change can't silently + // regress it. + let servedId: string | undefined; + const app = new H3({ allowMalformedURL: true }).all("/**", (event) => + serveStatic(event, { + getContents: (id) => { + servedId = id; + return `asset:${id}`; + }, + getMeta: () => ({ size: 1 }), + }), + ); + + const res = await app.request("/a%ZZ/../..%5c..%5cwindows%5cwin.ini"); + expect(res.status).toBe(200); + expect(servedId).toBeDefined(); + expect(servedId).not.toContain("\\"); // no raw backslash separator + expect(servedId).not.toMatch(/(^|[\\/])\.\.([\\/]|$)/); // no bare `..` segment + }); +});