Skip to content

security(redact-url): URIError on malformed %-encoding may leak query params to logs #127

@CryptoJones

Description

@CryptoJones

Problem

app/middleware/redact-url.js:47 calls decodeURIComponent(rawName) on every query parameter name without try-wrapping. decodeURIComponent throws URIError: URI malformed whenever it sees invalid percent-encoding:

  • ?%FF=bar%FF is an incomplete UTF-8 sequence
  • ?%ZZ=bar%ZZ isn't valid hex at all

redactUrl is invoked by pino-http's request serializer for every incoming request (see server.js). When that serializer throws, pino's fallback behavior is to either skip the log line entirely or log the raw value — in the latter case, the very authkey=… / token=… / password=… query param the redaction step was meant to strip is now in the structured log.

Repro:

const { redactUrl } = require('./app/middleware/redact-url.js');
redactUrl('/v1/foo?%FF=bar');
// → URIError: URI malformed

Worth noting: an attacker who knows about this can craft requests with a malformed percent-encoding ahead of a real authkey= param to disrupt redaction logging. Even without an attacker, ordinary buggy clients can trip it accidentally.

Proposed fix

Wrap the decodeURIComponent call in a try/catch; on URIError, fall back to lowercasing the raw (still-encoded) name. The raw bytes are preserved in the output either way, so no value is lost. A percent-malformed name almost certainly isn't a real entry in the sensitive-param allowlist.

Pin the behavior with a regression test in tests/unit/redact-url.test.js.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions