Horton

Thread

Commits

  1. Fix corner case bug in numeric to_char() some more.

  1. BUG #17839: Heap-buffer overflow on float8_to_char with invalid template

    The Post Office <noreply@postgresql.org> — 2023-03-14T01:51:09Z

    The following bug has been logged on the website:

Bug reference:      17839
Logged by:          Thiago Nunes
Email address:      thiagotnunes@gmail.com
PostgreSQL version: 15.2
Operating system:   Linux
Description:        

Heap-buffer overflow on float8_to_char when format exceeds max double
digits. I noticed this when running tests with memory sanitiser (msan).

The following example triggers the failure (considering max double digits
`DBL_DIG` is 15):

```
float8_to_char(12345678901, "FM9999999999D999990")
```

Explanation below:

1. After parsing the format, `Num.pre` will be 10, `Num.post` will be 6
`Num.zero_end` will be 16
(https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L1196-L1228)
2. The template size is greater than the `DBL_DIG`, `Num.post` will be moved
back here
(https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L6688-L6689).
3. The shortened template with the max `DBL_DIG` will be "stringfied" out
here
(https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L6712-L6717).
The result will be "##########.####" (10 significant digits + '.' + 4
decimal digits).
4. `Np->last_relevant` will be lesser than `Num->zero_end`, so it is updated
to an invalid position in the result above (pointer + 16) here
(https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L5740-L5743).
5. When applying FILLMODE here
(https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L5563),
it will try to get the character at Np->last_relevant, which is out of
bounds.

  2. Re: BUG #17839: Heap-buffer overflow on float8_to_char with invalid template

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-14T22:49:05Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> Heap-buffer overflow on float8_to_char when format exceeds max double
> digits. I noticed this when running tests with memory sanitiser (msan).
> The following example triggers the failure (considering max double digits
> `DBL_DIG` is 15):
> float8_to_char(12345678901, "FM9999999999D999990")

Thanks for the report!  For reasons that aren't entirely clear to me,
valgrind didn't whine about this until I added a few more zeroes to the
format string, like

SELECT to_char('12345678901'::float8, 'FM9999999999D9999900000000000000000');

Nonetheless, it did see the issue at that point.  I'm inclined to fix
it by making the code that adjusts Np->last_relevant more paranoid,
as attached.

			regards, tom lane

  3. Re: BUG #17839: Heap-buffer overflow on float8_to_char with invalid template

    Thiago Nunes <thiagotnunes@gmail.com> — 2023-03-15T01:17:38Z

    Thanks Tom,

I think your solution deals with all the cases, but I would like to point
out how I fixed it locally. I recalculated Num.zero_end after this line (
https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L6716
):

```
Num.zero_end = Num.pre + Num.post;
```

Cheers,

On Wed, Mar 15, 2023 at 9:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> PG Bug reporting form <noreply@postgresql.org> writes:
> > Heap-buffer overflow on float8_to_char when format exceeds max double
> > digits. I noticed this when running tests with memory sanitiser (msan).
> > The following example triggers the failure (considering max double digits
> > `DBL_DIG` is 15):
> > float8_to_char(12345678901, "FM9999999999D999990")
>
> Thanks for the report!  For reasons that aren't entirely clear to me,
> valgrind didn't whine about this until I added a few more zeroes to the
> format string, like
>
> SELECT to_char('12345678901'::float8,
> 'FM9999999999D9999900000000000000000');
>
> Nonetheless, it did see the issue at that point.  I'm inclined to fix
> it by making the code that adjusts Np->last_relevant more paranoid,
> as attached.
>
>                         regards, tom lane
>
>

  4. Re: BUG #17839: Heap-buffer overflow on float8_to_char with invalid template

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-15T01:45:46Z

    Thiago Nunes <thiagotnunes@gmail.com> writes:
> I think your solution deals with all the cases, but I would like to point
> out how I fixed it locally. I recalculated Num.zero_end after this line (
> https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L6716
> ):

> ```
> Num.zero_end = Num.pre + Num.post;
> ```

Hmm ... that seems a bit ad-hoc, because as far as I understand this
code, zero_end is supposed to track where is the last '0' format
character.  That shouldn't change just because we decided that the
data value overflowed.

			regards, tom lane

  5. Re: BUG #17839: Heap-buffer overflow on float8_to_char with invalid template

    Thiago Nunes <thiagotnunes@gmail.com> — 2023-03-15T02:02:37Z

    Got it, I thought that since we were filling the string with #s there, we
should reposition the zero_end.

I will wait for your patch then, thanks for looking into it!

Cheers,

On Wed, Mar 15, 2023 at 12:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Thiago Nunes <thiagotnunes@gmail.com> writes:
> > I think your solution deals with all the cases, but I would like to point
> > out how I fixed it locally. I recalculated Num.zero_end after this line (
> >
> https://github.com/postgres/postgres/blob/REL_15_2/src/backend/utils/adt/formatting.c#L6716
> > ):
>
> > ```
> > Num.zero_end = Num.pre + Num.post;
> > ```
>
> Hmm ... that seems a bit ad-hoc, because as far as I understand this
> code, zero_end is supposed to track where is the last '0' format
> character.  That shouldn't change just because we decided that the
> data value overflowed.
>
>                         regards, tom lane
>