BUG #17839: Heap-buffer overflow on float8_to_char with invalid template
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.
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
Attachments:
fix-wild-read-in-to_char.patchtext/x-diff; charset=us-ascii; name=fix-wild-read-in-to_char.patchDownload+16-2
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:
Show quoted text
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, likeSELECT 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
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
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:
Show quoted text
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