From 2786bd98a5363849f5641b624469251800e677f0 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Fri, 13 Nov 2026 18:37:35 -0800 Subject: [PATCH] Querystring: fix decoding after invalid UTF-8 Previously, percent-decoding advanced past each encoded byte even when the UTF-8 decoder left it unconsumed after reporting an invalid pending sequence. For example, "%D8%41%42" decoded to "\ufffdB" instead of "\ufffdAB". This affected querystring.parse() and unescape() in both engines, and r.args in nginx. Now njs_utf8_consume() reports whether the byte was consumed, so callers can reread it before advancing the input, as the WHATWG UTF-8 decoder requires. Plus signs are also passed through the decoder as spaces, so they flush invalid pending sequences without losing or reordering the replacement character. The issue was introduced in 50ea0dbc (0.4.3), copied into QuickJS r.args in 201b1276 (0.8.6) and into its querystring module in f289dcb9 (0.8.10). --- external/njs_query_string_module.c | 14 ++++++++++---- external/qjs_query_string_module.c | 14 ++++++++++---- nginx/ngx_http_js_module.c | 14 ++++++++++---- nginx/t/js_args.t | 14 +++++++++++++- src/njs_utf8.h | 10 ++++++++-- test/querystring.t.mjs | 13 +++++++++++++ 6 files changed, 64 insertions(+), 15 deletions(-) diff --git a/external/njs_query_string_module.c b/external/njs_query_string_module.c index c5b4d58a..94267317 100644 --- a/external/njs_query_string_module.c +++ b/external/njs_query_string_module.c @@ -127,6 +127,7 @@ njs_query_string_decode(njs_vm_t *vm, njs_value_t *value, const u_char *start, u_char *dst; uint32_t cp; njs_int_t ret; + njs_bool_t consumed; njs_chb_t chain; const u_char *p, *end; njs_unicode_decode_t ctx; @@ -162,13 +163,18 @@ njs_query_string_decode(njs_vm_t *vm, njs_value_t *value, const u_char *start, while (p < end) { if (*p == '%' && end - p > 2 && hex[p[1]] >= 0 && hex[p[2]] >= 0) { - cp = njs_utf8_consume(&ctx, (hex[p[1]] << 4) | hex[p[2]]); - p += 3; + cp = njs_utf8_consume(&ctx, (hex[p[1]] << 4) | hex[p[2]], + &consumed); + if (consumed) { + p += 3; + } } else { if (*p == '+') { - cp = ' '; - p++; + cp = njs_utf8_consume(&ctx, ' ', &consumed); + if (consumed) { + p++; + } } else { cp = njs_utf8_decode(&ctx, &p, end); diff --git a/external/qjs_query_string_module.c b/external/qjs_query_string_module.c index 128a17cd..b3b429b2 100644 --- a/external/qjs_query_string_module.c +++ b/external/qjs_query_string_module.c @@ -172,6 +172,7 @@ qjs_query_string_decode(JSContext *cx, const u_char *start, size_t size) { u_char *dst; uint32_t cp; + njs_bool_t consumed; njs_chb_t chain; const u_char *p, *end; njs_unicode_decode_t ctx; @@ -207,13 +208,18 @@ qjs_query_string_decode(JSContext *cx, const u_char *start, size_t size) while (p < end) { if (*p == '%' && end - p > 2 && hex[p[1]] >= 0 && hex[p[2]] >= 0) { - cp = njs_utf8_consume(&ctx, (hex[p[1]] << 4) | hex[p[2]]); - p += 3; + cp = njs_utf8_consume(&ctx, (hex[p[1]] << 4) | hex[p[2]], + &consumed); + if (consumed) { + p += 3; + } } else { if (*p == '+') { - cp = ' '; - p++; + cp = njs_utf8_consume(&ctx, ' ', &consumed); + if (consumed) { + p++; + } } else { cp = njs_utf8_decode(&ctx, &p, end); diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 4f9ac9b2..2b7109ed 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -6120,6 +6120,7 @@ ngx_http_qjs_query_string_decode(njs_chb_t *chain, const u_char *start, { u_char *dst; uint32_t cp; + njs_bool_t consumed; const u_char *p, *end; njs_unicode_decode_t ctx; @@ -6153,13 +6154,18 @@ ngx_http_qjs_query_string_decode(njs_chb_t *chain, const u_char *start, while (p < end) { if (*p == '%' && end - p > 2 && hex[p[1]] >= 0 && hex[p[2]] >= 0) { - cp = njs_utf8_consume(&ctx, (hex[p[1]] << 4) | hex[p[2]]); - p += 3; + cp = njs_utf8_consume(&ctx, (hex[p[1]] << 4) | hex[p[2]], + &consumed); + if (consumed) { + p += 3; + } } else { if (*p == '+') { - cp = ' '; - p++; + cp = njs_utf8_consume(&ctx, ' ', &consumed); + if (consumed) { + p++; + } } else { cp = njs_utf8_decode(&ctx, &p, end); diff --git a/nginx/t/js_args.t b/nginx/t/js_args.t index bb5cf4ff..0adfddd8 100644 --- a/nginx/t/js_args.t +++ b/nginx/t/js_args.t @@ -94,7 +94,7 @@ $t->write_file('test.js', <try_run('no njs')->plan(15); +$t->try_run('no njs')->plan(20); ############################################################################### @@ -145,6 +145,18 @@ is(get_json('/object?a=%6&b=%&c=%%&d=%zz'), '{"a":"%6","b":"%","c":"%%","d":"%zz"}', 'values percent-encoded broken object'); +is(get_json('/object?%D8%41%42=%E2%82%41%42'), + qq/{"\x{fffd}AB":"\x{fffd}AB"}/, + 'percent-encoded units after invalid UTF-8'); +is(get_json('/object?a=%D8+%41'), qq/{"a":"\x{fffd} A"}/, + 'plus after invalid UTF-8'); +is(get_json('/object?a=%E2%82+'), qq/{"a":"\x{fffd} "}/, + 'final plus after invalid UTF-8'); +is(get_json('/object?a=%D8%C2%A2'), qq/{"a":"\x{fffd}\x{a2}"}/, + 'valid UTF-8 after invalid UTF-8'); +is(get_json('/object?a=%ED%A0%80'), qq/{"a":"\x{fffd}\x{fffd}\x{fffd}"}/, + 'UTF-8 encoded surrogate'); + ############################################################################### sub has_version { diff --git a/src/njs_utf8.h b/src/njs_utf8.h index 5f2f81e2..7386b4b7 100644 --- a/src/njs_utf8.h +++ b/src/njs_utf8.h @@ -27,13 +27,19 @@ NJS_EXPORT njs_bool_t njs_utf8_is_valid(const u_char *p, size_t len); njs_inline uint32_t -njs_utf8_consume(njs_unicode_decode_t *ctx, u_char byte) +njs_utf8_consume(njs_unicode_decode_t *ctx, u_char byte, njs_bool_t *consumed) { + uint32_t cp; const u_char *p; p = &byte; - return njs_utf8_decode(ctx, &p, p + 1); + cp = njs_utf8_decode(ctx, &p, p + 1); + + /* An error may leave the byte to be decoded again. */ + *consumed = (p != &byte); + + return cp; } diff --git a/test/querystring.t.mjs b/test/querystring.t.mjs index 8ea14050..d9ebc0a3 100644 --- a/test/querystring.t.mjs +++ b/test/querystring.t.mjs @@ -117,6 +117,13 @@ let parse_tsuite = { { value: 'baz=%F', expected: { baz:'%F' } }, { value: 'baz=%', expected: { baz:'%' } }, { value: 'ba+z=f+uz', expected: { 'ba z':'f uz' } }, + { value: '%D8%41%42=%E2%82%41%42', + expected: { '�AB':'�AB' } }, + { value: 'baz=%D8+%41', expected: { baz:'� A' } }, + { value: 'baz=%E2%82+', expected: { baz:'� ' } }, + { value: 'baz=%D8%C2%A2', expected: { baz:'�¢' } }, + { value: 'baz=%D8%F0%9F%92%A9', expected: { baz:'�💩' } }, + { value: 'baz=%ED%A0%80', expected: { baz:'���' } }, { value: 'X=' + 'α'.repeat(33), expected: { X:'α'.repeat(33) } }, { value: 'X=' + 'α1'.repeat(33), expected: { X:'α1'.repeat(33) } }, @@ -214,6 +221,12 @@ let unescape_tsuite = { { value: '', expected: '' }, { value: 'baz%3Dfuz', expected: 'baz=fuz' }, { value: 'abc%CE%B1def', expected: 'abcαdef' }, + { value: '%D8%41%42', expected: '�AB' }, + { value: '%E2%41%42', expected: '�AB' }, + { value: '%E2%82%41%42', expected: '�AB' }, + { value: '%D8%C2%A2', expected: '�¢' }, + { value: '%D8%F0%9F%92%A9', expected: '�💩' }, + { value: '%ED%A0%80', expected: '���' }, ]}; run([ -- 2.47.3