diff --git a/app/middleware/redact-url.js b/app/middleware/redact-url.js index e1f214b..8555216 100644 --- a/app/middleware/redact-url.js +++ b/app/middleware/redact-url.js @@ -44,7 +44,20 @@ function redactUrl(url) { if (part.length === 0) continue; const eq = part.indexOf('='); const rawName = eq === -1 ? part : part.slice(0, eq); - const name = decodeURIComponent(rawName).toLowerCase(); + // decodeURIComponent throws URIError on malformed percent-encoding + // (e.g. `?%FF=v` or `?%ZZ=v`). pino-http calls this serializer for + // every request, so an unhandled throw here would either skip the + // log line entirely or fall back to logging the raw URL — leaking + // the very value we're trying to redact. Wrap and treat + // undecodable names as non-sensitive (the raw bytes are kept in + // the output, so no value is lost; a malformed name almost + // certainly isn't a real `authkey=…` anyway). + let name; + try { + name = decodeURIComponent(rawName).toLowerCase(); + } catch (_err) { + name = rawName.toLowerCase(); + } if (SENSITIVE_PARAM_NAMES.has(name)) { out.push(`${rawName}=`); } else { diff --git a/tests/unit/redact-url.test.js b/tests/unit/redact-url.test.js index 6ee0112..ac91e51 100644 --- a/tests/unit/redact-url.test.js +++ b/tests/unit/redact-url.test.js @@ -69,4 +69,19 @@ describe('redactUrl', () => { expect(redactUrl(null)).toBe(null); expect(redactUrl(42)).toBe(42); }); + + test('does not throw on malformed percent-encoding in param name', () => { + // decodeURIComponent throws URIError on `%FF` (incomplete UTF-8 + // sequence) or `%ZZ` (invalid hex). pino-http would invoke this + // serializer once per request, so an unhandled URIError here would + // either skip the log line or fall back to logging the raw URL — + // leaking the very value we're meant to redact. Wrap + recover. + expect(() => redactUrl('/x?%FF=bar')).not.toThrow(); + expect(() => redactUrl('/x?%ZZ=bar&authkey=secret')).not.toThrow(); + // And the sensitive param that follows the malformed one must + // still be redacted — recovery doesn't abort the loop. + const out = redactUrl('/x?%ZZ=bar&authkey=secret'); + expect(out).not.toContain('secret'); + expect(out).toContain('authkey='); + }); });