Skip to content

fix(sirv): clamp Range end above MAX_SAFE_INTEGER to avoid crash#175

Open
maciejdzierzek wants to merge 1 commit into
lukeed:mainfrom
maciejdzierzek:fix/range-end-clamp-and-overflow
Open

fix(sirv): clamp Range end above MAX_SAFE_INTEGER to avoid crash#175
maciejdzierzek wants to merge 1 commit into
lukeed:mainfrom
maciejdzierzek:fix/range-end-clamp-and-overflow

Conversation

@maciejdzierzek
Copy link
Copy Markdown

Summary

A malformed Range header whose end value exceeds Number.MAX_SAFE_INTEGER (e.g. bytes=0-18446744073709551615 — easy to produce from bots / fuzz traffic) currently crashes the process:

RangeError [ERR_OUT_OF_RANGE]: The value of "end" is out of range.
It must be >= 0 && <= 9007199254740991. Received 18_446_744_073_709_552_000
    at new ReadStream (node:internal/fs/streams:217:5)
    at send (packages/sirv/index.mjs:95:5)

This hit production for me through @sveltejs/adapter-node (which uses sirv for static assets) — single bot probe took the whole worker down. Reproduced verbatim against this repo's test suite on the current main.

Root cause

In send():

let end = opts.end = parseInt(y, 10) || stats.size - 1;  // opts.end = 1.8e19
// ...
if (end >= stats.size) {
    end = stats.size - 1;                                 // only the lexical `end` is clamped
}
// ...
fs.createReadStream(file, opts).pipe(res);                // opts.end still 1.8e19 → throws

The clamp re-binds the lexical end but not opts.end, so the original (out-of-range) value still reaches fs.createReadStream, which validates against MAX_SAFE_INTEGER synchronously and throws.

This is the same class as #140 / #147; that fix landed in the CJS packages/sirv/index.js source in 2023 but was lost during the 2024-10 ESM-only refactor (f0113f3).

Fix

  • Reject non-safe-integer parses up front and fall back to start=0 / end=stats.size-1 (per RFC 7233 §3.1: malformed Range MAY be ignored, treated as absent).
  • Re-assign opts.end together with the lexical end when the file-size clamp fires.
  • Tighten the 416 branch with a start > end check so swapped ranges can't reach createReadStream with a negative-length window.

Tests

Three new tests in the existing ranges suite:

  • should clamp range end above MAX_SAFE_INTEGER without crashing — the regression test (reproduces the reported crash on unpatched code).
  • should fall back to a full-file response when range start is above MAX_SAFE_INTEGER.
  • should treat a non-numeric range as a full-file request — documents existing behavior, kept stable.

Verification

  • All existing sirv tests pass (76 → 79, +3 new).
  • Verified the regression test fails on unpatched main with the exact RangeError ERR_OUT_OF_RANGE 18_446_744_073_709_552_000 — same fingerprint as the production crash.
  • The sirv-cli help suite has a pre-existing failure on main unrelated to this change.

A malformed `Range` header whose end value exceeds Number.MAX_SAFE_INTEGER
(e.g. `bytes=0-18446744073709551615`, easily produced by bots / fuzz
traffic) crashed the process with:

  RangeError [ERR_OUT_OF_RANGE]: The value of "end" is out of range.
  It must be >= 0 && <= 9007199254740991.
      at new ReadStream (node:internal/fs/streams:217:5)
      at send (packages/sirv/index.mjs:95:5)

Root cause: although the local `end` variable was clamped to
`stats.size - 1` when the parsed value exceeded the file size, the
clamp did not update `opts.end`, which was assigned in the same
declaration. The original (out-of-range) value was therefore still
passed to `fs.createReadStream({ end })`, which validates synchronously
against `Number.MAX_SAFE_INTEGER` and throws.

This is the same class of bug as lukeed#140 / lukeed#147; that fix landed in the
CJS `index.js` source in 2023 but was lost during the 2024-10
ESM-only refactor (commit f0113f3).

Changes:

- Reject non-safe-integer parses up front (treat as "absent" per
  RFC 7233 §3.1, falling back to start=0 / end=stats.size-1).
- Re-assign `opts.end` together with the lexical `end` when the
  clamp fires, so `createReadStream` always receives a safe value.
- Add a `start > end` check to the 416 branch so a swapped range
  cannot reach `createReadStream` with negative-length opts.

Tests:

- Regression test for `bytes=0-18446744073709551615` (reproduces the
  reported crash on the unpatched code).
- Range start above MAX_SAFE_INTEGER falls back to a full-file 206.
- Non-numeric range tokens (`bytes=abc-def`) fall back to a full-file
  206 — documents existing behavior, kept stable.

Verified by reverting `index.mjs` and observing the suite crash with
the same `RangeError` reported in production, then re-applying the
fix to confirm all 13 range tests (including the 3 new ones) pass.
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.

1 participant