pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Fix overread in JSON parsing errors for incomplete byte sequence
12+ messages / 4 participants
[nested] [flat]
* pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-09 03:46 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-05-09 03:46 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix overread in JSON parsing errors for incomplete byte sequences
json_lex_string() relies on pg_encoding_mblen_bounded() to point to the
end of a JSON string when generating an error message, and the input it
uses is not guaranteed to be null-terminated.
It was possible to walk off the end of the input buffer by a few bytes
when the last bytes consist of an incomplete multi-byte sequence, as
token_terminator would point to a location defined by
pg_encoding_mblen_bounded() rather than the end of the input. This
commit switches token_terminator so as the error uses data up to the
end of the JSON input.
More work should be done so as this code could rely on an equivalent of
report_invalid_encoding() so as incorrect byte sequences can show in
error messages in a readable form. This requires work for at least two
cases in the JSON parsing API: an incomplete token and an invalid escape
sequence. A more complete solution may be too invasive for a backpatch,
so this is left as a future improvement, taking care of the overread
first.
A test is added on HEAD as test_json_parser makes this issue
straight-forward to check.
Note that pg_encoding_mblen_bounded() no longer has any callers. This
will be removed on HEAD with a separate commit, as this is proving to
encourage unsafe coding.
Author: Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com
Backpatch-through: 13
Branch
------
REL_15_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/8c3f30e675e972e3c2750ea82781f2c4ba77a9f8
Modified Files
--------------
src/common/jsonapi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-09 03:46 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-05-09 03:46 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix overread in JSON parsing errors for incomplete byte sequences
json_lex_string() relies on pg_encoding_mblen_bounded() to point to the
end of a JSON string when generating an error message, and the input it
uses is not guaranteed to be null-terminated.
It was possible to walk off the end of the input buffer by a few bytes
when the last bytes consist of an incomplete multi-byte sequence, as
token_terminator would point to a location defined by
pg_encoding_mblen_bounded() rather than the end of the input. This
commit switches token_terminator so as the error uses data up to the
end of the JSON input.
More work should be done so as this code could rely on an equivalent of
report_invalid_encoding() so as incorrect byte sequences can show in
error messages in a readable form. This requires work for at least two
cases in the JSON parsing API: an incomplete token and an invalid escape
sequence. A more complete solution may be too invasive for a backpatch,
so this is left as a future improvement, taking care of the overread
first.
A test is added on HEAD as test_json_parser makes this issue
straight-forward to check.
Note that pg_encoding_mblen_bounded() no longer has any callers. This
will be removed on HEAD with a separate commit, as this is proving to
encourage unsafe coding.
Author: Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com
Backpatch-through: 13
Branch
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/41adf9d960c0105ada615574126d5568d552f4db
Modified Files
--------------
src/common/jsonapi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-09 03:46 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-05-09 03:46 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix overread in JSON parsing errors for incomplete byte sequences
json_lex_string() relies on pg_encoding_mblen_bounded() to point to the
end of a JSON string when generating an error message, and the input it
uses is not guaranteed to be null-terminated.
It was possible to walk off the end of the input buffer by a few bytes
when the last bytes consist of an incomplete multi-byte sequence, as
token_terminator would point to a location defined by
pg_encoding_mblen_bounded() rather than the end of the input. This
commit switches token_terminator so as the error uses data up to the
end of the JSON input.
More work should be done so as this code could rely on an equivalent of
report_invalid_encoding() so as incorrect byte sequences can show in
error messages in a readable form. This requires work for at least two
cases in the JSON parsing API: an incomplete token and an invalid escape
sequence. A more complete solution may be too invasive for a backpatch,
so this is left as a future improvement, taking care of the overread
first.
A test is added on HEAD as test_json_parser makes this issue
straight-forward to check.
Note that pg_encoding_mblen_bounded() no longer has any callers. This
will be removed on HEAD with a separate commit, as this is proving to
encourage unsafe coding.
Author: Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com
Backpatch-through: 13
Branch
------
REL_16_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/5396a2987cc679a3ab5db26b54c8b76149489e29
Modified Files
--------------
src/common/jsonapi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-09 03:46 Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-05-09 03:46 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix overread in JSON parsing errors for incomplete byte sequences
json_lex_string() relies on pg_encoding_mblen_bounded() to point to the
end of a JSON string when generating an error message, and the input it
uses is not guaranteed to be null-terminated.
It was possible to walk off the end of the input buffer by a few bytes
when the last bytes consist of an incomplete multi-byte sequence, as
token_terminator would point to a location defined by
pg_encoding_mblen_bounded() rather than the end of the input. This
commit switches token_terminator so as the error uses data up to the
end of the JSON input.
More work should be done so as this code could rely on an equivalent of
report_invalid_encoding() so as incorrect byte sequences can show in
error messages in a readable form. This requires work for at least two
cases in the JSON parsing API: an incomplete token and an invalid escape
sequence. A more complete solution may be too invasive for a backpatch,
so this is left as a future improvement, taking care of the overread
first.
A test is added on HEAD as test_json_parser makes this issue
straight-forward to check.
Note that pg_encoding_mblen_bounded() no longer has any callers. This
will be removed on HEAD with a separate commit, as this is proving to
encourage unsafe coding.
Author: Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com
Backpatch-through: 13
Branch
------
REL_13_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/377c25d32275a65043f9dcf3d4668a56c37319e2
Modified Files
--------------
src/common/jsonapi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-09 03:46 Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2024-05-09 03:46 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix overread in JSON parsing errors for incomplete byte sequences
json_lex_string() relies on pg_encoding_mblen_bounded() to point to the
end of a JSON string when generating an error message, and the input it
uses is not guaranteed to be null-terminated.
It was possible to walk off the end of the input buffer by a few bytes
when the last bytes consist of an incomplete multi-byte sequence, as
token_terminator would point to a location defined by
pg_encoding_mblen_bounded() rather than the end of the input. This
commit switches token_terminator so as the error uses data up to the
end of the JSON input.
More work should be done so as this code could rely on an equivalent of
report_invalid_encoding() so as incorrect byte sequences can show in
error messages in a readable form. This requires work for at least two
cases in the JSON parsing API: an incomplete token and an invalid escape
sequence. A more complete solution may be too invasive for a backpatch,
so this is left as a future improvement, taking care of the overread
first.
A test is added on HEAD as test_json_parser makes this issue
straight-forward to check.
Note that pg_encoding_mblen_bounded() no longer has any callers. This
will be removed on HEAD with a separate commit, as this is proving to
encourage unsafe coding.
Author: Jacob Champion
Discussion: https://postgr.es/m/CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com
Backpatch-through: 13
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/855517307db8efd397d49163d65a4fd3bdcc41bc
Modified Files
--------------
src/common/jsonapi.c | 4 ++--
src/test/modules/test_json_parser/t/002_inline.pl | 8 ++++++++
2 files changed, 10 insertions(+), 2 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-10 11:59 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2024-05-10 11:59 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org, Jacob Champion <jacob.champion@enterprisedb.com>
On 2024-May-09, Michael Paquier wrote:
> Fix overread in JSON parsing errors for incomplete byte sequences
I'm getting this error in the new test:
t/002_inline.pl ........................ 1/?
# Failed test 'incomplete UTF-8 sequence, chunk size 3: correct error output'
# at t/002_inline.pl line 134.
# 'Escape sequence "\�1+2" is invalid.'
# doesn't match '(?^:(Token|Escape sequence) ""?\\\x{F5}" is invalid)'
# Looks like you failed 1 test of 850.
Not sure what's going on here, or why it fails for me while the
buildfarm is all happy.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Every machine is a smoke machine if you operate it wrong enough."
https://twitter.com/libseybieda/status/1541673325781196801
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-10 12:23 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 2 replies; 12+ messages in thread
From: Alvaro Herrera @ 2024-05-10 12:23 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org, Jacob Champion <jacob.champion@enterprisedb.com>
On 2024-May-10, Alvaro Herrera wrote:
> Not sure what's going on here, or why it fails for me while the
> buildfarm is all happy.
Ah, I ran 'git clean -dfx' and now it works correctly. I must have had
an incomplete rebuild.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"World domination is proceeding according to plan" (Andrew Morton)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-13 03:22 Michael Paquier <michael@paquier.xyz>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
1 sibling, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-05-13 03:22 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers@lists.postgresql.org, Jacob Champion <jacob.champion@enterprisedb.com>
On Fri, May 10, 2024 at 02:23:09PM +0200, Alvaro Herrera wrote:
> Ah, I ran 'git clean -dfx' and now it works correctly. I must have had
> an incomplete rebuild.
I am going to assume that this is an incorrect build. It seems to me
that src/common/ was compiled with a past version not sufficient to
make the new test pass as more bytes got pushed to the error output as
the pre-855517307db8 code could point to some random junk.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-14 08:39 Peter Eisentraut <peter@eisentraut.org>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
1 sibling, 1 reply; 12+ messages in thread
From: Peter Eisentraut @ 2024-05-14 08:39 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org, Jacob Champion <jacob.champion@enterprisedb.com>
On 10.05.24 14:23, Alvaro Herrera wrote:
> On 2024-May-10, Alvaro Herrera wrote:
>
>> Not sure what's going on here, or why it fails for me while the
>> buildfarm is all happy.
>
> Ah, I ran 'git clean -dfx' and now it works correctly. I must have had
> an incomplete rebuild.
I saw the same thing. The problem is that there is incomplete
dependency information in the makefiles (not meson) between src/common/
and what is using it. So whenever anything changes in src/common/, you
pretty much have to do a forced rebuild of everything.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-15 00:00 Michael Paquier <michael@paquier.xyz>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2024-05-15 00:00 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Jacob Champion <jacob.champion@enterprisedb.com>
On Tue, May 14, 2024 at 10:39:36AM +0200, Peter Eisentraut wrote:
> I saw the same thing. The problem is that there is incomplete dependency
> information in the makefiles (not meson) between src/common/ and what is
> using it. So whenever anything changes in src/common/, you pretty much have
> to do a forced rebuild of everything.
Is that a recent regression? I have some blurry memories from
working on these areas that changing src/common/ reflected on the
compiled pieces effectively at some point.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-15 06:15 Peter Eisentraut <peter@eisentraut.org>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 12+ messages in thread
From: Peter Eisentraut @ 2024-05-15 06:15 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Jacob Champion <jacob.champion@enterprisedb.com>
On 15.05.24 02:00, Michael Paquier wrote:
> On Tue, May 14, 2024 at 10:39:36AM +0200, Peter Eisentraut wrote:
>> I saw the same thing. The problem is that there is incomplete dependency
>> information in the makefiles (not meson) between src/common/ and what is
>> using it. So whenever anything changes in src/common/, you pretty much have
>> to do a forced rebuild of everything.
>
> Is that a recent regression? I have some blurry memories from
> working on these areas that changing src/common/ reflected on the
> compiled pieces effectively at some point.
One instance of this problem that I can reproduce at least back to PG12 is
1. touch src/common/exec.c
2. make -C src/bin/pg_dump
This will rebuild libpgcommon, but it will not relink pg_dump.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Fix overread in JSON parsing errors for incomplete byte sequence
@ 2024-05-15 18:22 Jacob Champion <jacob.champion@enterprisedb.com>
parent: Peter Eisentraut <peter@eisentraut.org>
0 siblings, 0 replies; 12+ messages in thread
From: Jacob Champion @ 2024-05-15 18:22 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Michael Paquier <michael@paquier.xyz>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org
On Tue, May 14, 2024 at 11:15 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 15.05.24 02:00, Michael Paquier wrote:
> > Is that a recent regression? I have some blurry memories from
> > working on these areas that changing src/common/ reflected on the
> > compiled pieces effectively at some point.
>
> One instance of this problem that I can reproduce at least back to PG12 is
>
> 1. touch src/common/exec.c
> 2. make -C src/bin/pg_dump
>
> This will rebuild libpgcommon, but it will not relink pg_dump.
I remember src/common/unicode changes having similar trouble, as well [1].
--Jacob
[1] https://www.postgresql.org/message-id/CAFBsxsGZTwzDnTs=TVM38CCTPP3Y0D3=h+UiWt8M83D5THHf9A@mail.gmail...
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2024-05-15 18:22 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-05-09 03:46 pgsql: Fix overread in JSON parsing errors for incomplete byte sequence Michael Paquier <michael@paquier.xyz>
2024-05-10 11:59 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2024-05-10 12:23 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2024-05-13 03:22 ` Michael Paquier <michael@paquier.xyz>
2024-05-14 08:39 ` Peter Eisentraut <peter@eisentraut.org>
2024-05-15 00:00 ` Michael Paquier <michael@paquier.xyz>
2024-05-15 06:15 ` Peter Eisentraut <peter@eisentraut.org>
2024-05-15 18:22 ` Jacob Champion <jacob.champion@enterprisedb.com>
2024-05-09 03:46 pgsql: Fix overread in JSON parsing errors for incomplete byte sequence Michael Paquier <michael@paquier.xyz>
2024-05-09 03:46 pgsql: Fix overread in JSON parsing errors for incomplete byte sequence Michael Paquier <michael@paquier.xyz>
2024-05-09 03:46 pgsql: Fix overread in JSON parsing errors for incomplete byte sequence Michael Paquier <michael@paquier.xyz>
2024-05-09 03:46 pgsql: Fix overread in JSON parsing errors for incomplete byte sequence Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox