From: Tom Lane <tgl@sss.pgh.pa.us>
To: thiagotnunes@gmail.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17839: Heap-buffer overflow on float8_to_char with invalid
 template
In-reply-to: <17839-aada50db24d7b0da@postgresql.org>
References: <17839-aada50db24d7b0da@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Tue, 14 Mar 2023 01:51:09 -0000"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <3011598.1678834101.0@sss.pgh.pa.us>
Date: Tue, 14 Mar 2023 18:49:05 -0400
Message-ID: <3011659.1678834145@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3011659.1678834145%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3011598.1678834101.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-wild-read-in-to_char.patch";
	charset="us-ascii"
Content-ID: <3011598.1678834101.2@sss.pgh.pa.us>
Content-Description: fix-wild-read-in-to_char.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/fo=
rmatting.c
index f3f4db5ef6..e6246dc44b 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -5695,13 +5695,20 @@ NUM_processor(FormatNode *node, NUMDesc *Num, char=
 *inout,
 =

 			/*
 			 * If any '0' specifiers are present, make sure we don't strip
-			 * those digits.
+			 * those digits.  But don't advance last_relevant beyond the last
+			 * character of the Np->number string, which is a hazard if the
+			 * number got shortened due to precision limitations.
 			 */
 			if (Np->last_relevant && Np->Num->zero_end > Np->out_pre_spaces)
 			{
+				int			last_zero_pos;
 				char	   *last_zero;
 =

-				last_zero =3D Np->number + (Np->Num->zero_end - Np->out_pre_spaces);
+				/* note that Np->number cannot be zero-length here */
+				last_zero_pos =3D strlen(Np->number) - 1;
+				last_zero_pos =3D Min(last_zero_pos,
+									Np->Num->zero_end - Np->out_pre_spaces);
+				last_zero =3D Np->number + last_zero_pos;
 				if (Np->last_relevant < last_zero)
 					Np->last_relevant =3D last_zero;
 			}
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expe=
cted/numeric.out
index 2ac2c99b19..26930f4db4 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1929,6 +1929,12 @@ SELECT to_char('100'::numeric, 'FM999');
  100
 (1 row)
 =

+SELECT to_char('12345678901'::float8, 'FM9999999999D999990000000000000000=
0');
+     to_char     =

+-----------------
+ ##########.####
+(1 row)
+
 -- Check parsing of literal text in a format string
 SELECT to_char('100'::numeric, 'foo999');
  to_char =

diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numer=
ic.sql
index 6e9e6145d0..2dddb58625 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -979,6 +979,7 @@ FROM v;
 SELECT to_char('100'::numeric, 'FM999.9');
 SELECT to_char('100'::numeric, 'FM999.');
 SELECT to_char('100'::numeric, 'FM999');
+SELECT to_char('12345678901'::float8, 'FM9999999999D999990000000000000000=
0');
 =

 -- Check parsing of literal text in a format string
 SELECT to_char('100'::numeric, 'foo999');

------- =_aaaaaaaaaa0--