[PATCH] Fix small overread during SASLprep

Started by Jacob Championover 1 year ago6 messageshackers
Jump to latest
#1Jacob Champion
jacob.champion@enterprisedb.com

Hi all,

pg_utf8_string_len() doesn't check the remaining string length before
calling pg_utf8_is_legal(), so there's a possibility of jumping a
couple of bytes past the end of the string. (The overread stops there,
because the function won't validate a sequence containing a null
byte.)

Here's a quick patch to fix it. I didn't see any other uses of
pg_utf8_is_legal() with missing length checks.

Thanks,
--Jacob

Attachments:

pg_utf8_string_len-honor-null-terminators.patchapplication/octet-stream; name=pg_utf8_string_len-honor-null-terminators.patchDownload+4-3
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Jacob Champion (#1)
Re: [PATCH] Fix small overread during SASLprep

On 9 Sep 2024, at 17:29, Jacob Champion <jacob.champion@enterprisedb.com> wrote:

pg_utf8_string_len() doesn't check the remaining string length before
calling pg_utf8_is_legal(), so there's a possibility of jumping a
couple of bytes past the end of the string. (The overread stops there,
because the function won't validate a sequence containing a null
byte.)

Here's a quick patch to fix it. I didn't see any other uses of
pg_utf8_is_legal() with missing length checks.

Just to make sure I understand, this is for guarding against overreads in
validation of strings containing torn MB characters? Assuming I didn't
misunderstand you this patch seems correct to me.

--
Daniel Gustafsson

#3Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Daniel Gustafsson (#2)
Re: [PATCH] Fix small overread during SASLprep

On Mon, Sep 9, 2024 at 11:30 AM Daniel Gustafsson <daniel@yesql.se> wrote:

Just to make sure I understand, this is for guarding against overreads in
validation of strings containing torn MB characters?

Right. Our SASLprep code doesn't require/enforce UTF8-encoded inputs.

Assuming I didn't
misunderstand you this patch seems correct to me.

Thanks for the review!

--Jacob

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Jacob Champion (#3)
Re: [PATCH] Fix small overread during SASLprep

On 9 Sep 2024, at 20:41, Jacob Champion <jacob.champion@enterprisedb.com> wrote:

On Mon, Sep 9, 2024 at 11:30 AM Daniel Gustafsson <daniel@yesql.se> wrote:

Just to make sure I understand, this is for guarding against overreads in
validation of strings containing torn MB characters?

Right. Our SASLprep code doesn't require/enforce UTF8-encoded inputs.

Thanks for confirming, I'll have another look in the morning and will apply
then unless there are objections.

--
Daniel Gustafsson

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Daniel Gustafsson (#4)
Re: [PATCH] Fix small overread during SASLprep

On 9 Sep 2024, at 23:21, Daniel Gustafsson <daniel@yesql.se> wrote:

On 9 Sep 2024, at 20:41, Jacob Champion <jacob.champion@enterprisedb.com> wrote:

On Mon, Sep 9, 2024 at 11:30 AM Daniel Gustafsson <daniel@yesql.se> wrote:

Just to make sure I understand, this is for guarding against overreads in
validation of strings containing torn MB characters?

Right. Our SASLprep code doesn't require/enforce UTF8-encoded inputs.

Thanks for confirming, I'll have another look in the morning and will apply
then unless there are objections.

Pushed, thanks!

--
Daniel Gustafsson

#6Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Daniel Gustafsson (#5)
Re: [PATCH] Fix small overread during SASLprep

On Tue, Sep 10, 2024 at 4:39 AM Daniel Gustafsson <daniel@yesql.se> wrote:

Pushed, thanks!

Thank you!

--Jacob