public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jacob Champion <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Subject: [PATCH] json_lex_string: don't overread on bad UTF8
Date: Tue, 30 Apr 2024 10:39:04 -0700
Message-ID: <CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com> (raw)
Hi all,
When json_lex_string() hits certain types of invalid input, it calls
pg_encoding_mblen_bounded(), which assumes that its input is
null-terminated and calls strnlen(). But the JSON lexer is constructed
with an explicit string length, and we don't ensure that the string is
null-terminated in all cases, so we can walk off the end of the
buffer. This isn't really relevant on the server side, where you'd
have to get a superuser to help you break string encodings, but for
client-side usage on untrusted input (such as my OAuth patch) it would
be more important.
Attached is a draft patch that explicitly checks against the
end-of-string pointer and clamps the token_terminator to it. Note that
this removes the only caller of pg_encoding_mblen_bounded() and I'm
not sure what we should do with that function. It seems like a
reasonable API, just not here.
The new test needs to record two versions of the error message, one
for invalid token and one for invalid escape sequence. This is
because, for smaller chunk sizes, the partial-token logic in the
incremental JSON parser skips the affected code entirely when it can't
find an ending double-quote.
Tangentially: Should we maybe rethink pieces of the json_lex_string
error handling? For example, do we really want to echo an incomplete
multibyte sequence once we know it's bad? It also looks like there are
places where the FAIL_AT_CHAR_END macro is called after the `s`
pointer has already advanced past the code point of interest. I'm not
sure if that's intentional.
Thanks,
--Jacob
Attachments:
[application/octet-stream] 0001-json_lex_string-don-t-overread-on-bad-UTF8.patch (1.9K, ../CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com/2-0001-json_lex_string-don-t-overread-on-bad-UTF8.patch)
download | inline diff:
From bba7744e59941d8bb2f039e631d090d0e3956d6c Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Mon, 8 Apr 2024 15:31:17 -0700
Subject: [PATCH] json_lex_string: don't overread on bad UTF8
Inputs to pg_parse_json[_incremental] are not guaranteed to be
null-terminated, so pg_encoding_mblen_bounded (which uses strnlen) can
walk off the end of the buffer. Check against the end pointer instead.
TODO:
- pg_encoding_mblen_bounded() now has no callers; should we remove it?
- Do we really want to print incomplete UTF-8 sequences as-is once we
know they're bad?
---
src/common/jsonapi.c | 5 +++--
src/test/modules/test_json_parser/t/002_inline.pl | 4 ++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index fc0cb36974..6633503490 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -1689,8 +1689,9 @@ json_lex_string(JsonLexContext *lex)
} while (0)
#define FAIL_AT_CHAR_END(code) \
do { \
- lex->token_terminator = \
- s + pg_encoding_mblen_bounded(lex->input_encoding, s); \
+ lex->token_terminator = s + pg_encoding_mblen(lex->input_encoding, s); \
+ if (lex->token_terminator >= end) \
+ lex->token_terminator = end; \
return code; \
} while (0)
diff --git a/src/test/modules/test_json_parser/t/002_inline.pl b/src/test/modules/test_json_parser/t/002_inline.pl
index f83cec03f8..0335a26f47 100644
--- a/src/test/modules/test_json_parser/t/002_inline.pl
+++ b/src/test/modules/test_json_parser/t/002_inline.pl
@@ -128,5 +128,9 @@ test(
"incorrect escape count",
'"\\\\\\\\\\\\\\"',
error => qr/Token ""\\\\\\\\\\\\\\"" is invalid/);
+test(
+ "incomplete UTF-8 sequence",
+ "\"\\\x{F5}", # three bytes: double-quote, backslash, <f5>
+ error => qr/(Token|Escape sequence) ""?\\\x{F5}" is invalid/);
done_testing();
--
2.34.1
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCH] json_lex_string: don't overread on bad UTF8
In-Reply-To: <CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox