Skip to content

Fix unhandled exception in request handling crashes the process - #153

Open
iaohkut-from-NightWolf-Team wants to merge 1 commit into
11ty:mainfrom
iaohkut-from-NightWolf-Team:fix/unhandled-exception-request-crash
Open

Fix unhandled exception in request handling crashes the process#153
iaohkut-from-NightWolf-Team wants to merge 1 commit into
11ty:mainfrom
iaohkut-from-NightWolf-Team:fix/unhandled-exception-request-crash

Conversation

@iaohkut-from-NightWolf-Team

Copy link
Copy Markdown

Fixes #150.

Summary

getOutputDirFilePath() throws Error("Invalid path") as its normal rejection path for an out-of-bounds request, but nothing in the call chain up to the raw http.Server request handler caught it. Since onRequestHandler is async, the thrown error becomes a rejected promise with no .catch() attached by the caller — an unhandled rejection that terminates the entire Node.js process on a single crafted GET request.

Fix

Wrap the middleware-chain invocation in onRequestHandler in a try/catch and respond with a normal 400 instead of letting the exception escape:

let [first] = bound;
try {
  await first();
} catch(e) {
  if(!res.headersSent) {
    res.statusCode = 400;
    res.end("Bad Request");
  }
}

Verification

  • Full existing test suite: 32/32 passing, no regressions.
  • PoC that previously crashed the server with a single request now returns 400, and the server stays alive for subsequent requests:
    $ curl http://localhost:PORT/%2e%2e%2fsecret.txt
    # before: process crashes, subsequent requests get connection refused
    # after:  HTTP 400, server stays up
    

See #150 for the full report and original reproduction.

…248/400)

getOutputDirFilePath() throws Error("Invalid path") as its normal rejection
path for an out-of-bounds request, but nothing in the call chain up to the
raw http.Server request handler caught it, so the exception propagated as
an unhandled promise rejection (onRequestHandler is async) and terminated
the entire Node.js process on a single crafted GET request.

Wrap the middleware-chain invocation in onRequestHandler in a try/catch and
respond with a normal 400 instead of letting the exception escape. Verified
against the existing test suite (32/32 passing) plus a PoC that previously
killed the server with one request and now returns 400 while the server
stays alive for subsequent requests.

Co-Authored-By: iaohkut <thb2601@gmail.com>
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.

Unauthenticated single-request DoS: unhandled exception in path validation crashes the entire process

2 participants