This is an automated email from the ASF dual-hosted git repository.
jrgemignani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/age.git
The following commit(s) were added to refs/heads/master by this push:
new fafdaa1c Fix stack overflow and precision loss in toFloatList()
conversion (#2451)
fafdaa1c is described below
commit fafdaa1c0ef5f4ab8ff09482f20a382b845dc9e5
Author: Greg Felice <[email protected]>
AuthorDate: Thu Jul 2 10:39:37 2026 -0400
Fix stack overflow and precision loss in toFloatList() conversion (#2451)
toFloatList()'s AGTV_FLOAT branch formatted each element with
sprintf(buffer, "%f", ...) into a fixed 64-byte stack buffer and then
re-parsed the string back into a float. This had two defects:
1. Stack overflow. "%f" prints the full integer part with no width
limit, so a large magnitude overflows the 64-byte buffer. The value
is query-reachable: RETURN toFloatList([1.0e308]) needs ~317 bytes
(309 integer digits + ".000000") and smashes the stack. This is the
issue reported in #2410.
2. Precision loss. "%f" emits only 6 fractional digits, so the
format-and-reparse round trip was lossy -- toFloatList([0.123456789])
returned 0.123457.
The element is already a float8, so the whole format/reparse step is
unnecessary. Assign elem->val.float_value directly. This removes the
stack buffer entirely (no magic buffer size to justify) and fixes both
the overflow and the precision loss at once.
Also harden toStringList(): its "%.*g"/"%ld" conversions use bounded
formats and were never overflow-prone, but switch them from sprintf to
snprintf as defensive depth.
Add regression coverage to regress/sql/expr.sql for both the large
magnitude case (no overflow) and precision preservation.
This reimplements the fix originally proposed by David Christensen in
#2410, whose report identified the sprintf overflow.
Co-authored-by: David Christensen <[email protected]>
---
regress/expected/expr.out | 20 ++++++++++++++++++++
regress/sql/expr.sql | 10 ++++++++++
src/backend/utils/adt/agtype.c | 19 +++++++++++--------
3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/regress/expected/expr.out b/regress/expected/expr.out
index d6b9ac15..806a6f65 100644
--- a/regress/expected/expr.out
+++ b/regress/expected/expr.out
@@ -3548,6 +3548,26 @@ $$) AS (toFloatList agtype);
[1.20002]
(1 row)
+-- large magnitudes must not overflow the conversion (regression: unbounded
+-- sprintf into a fixed stack buffer overflowed for values like 1.0e308)
+SELECT * FROM cypher('expr', $$
+ RETURN toFloatList([1.0e308, -1.0e308])
+$$) AS (toFloatList agtype);
+ tofloatlist
+-------------------
+ [1e+308, -1e+308]
+(1 row)
+
+-- precision must be preserved (regression: "%f" format truncated to 6 digits,
+-- so 0.123456789 came back as 0.123457)
+SELECT * FROM cypher('expr', $$
+ RETURN toFloatList([0.123456789])
+$$) AS (toFloatList agtype);
+ tofloatlist
+---------------
+ [0.123456789]
+(1 row)
+
-- should return null
SELECT * FROM cypher('expr', $$
RETURN toFloatList(['true'])
diff --git a/regress/sql/expr.sql b/regress/sql/expr.sql
index 251349ce..d4d900a1 100644
--- a/regress/sql/expr.sql
+++ b/regress/sql/expr.sql
@@ -1520,6 +1520,16 @@ $$) AS (toFloatList agtype);
SELECT * FROM cypher('expr', $$
RETURN toFloatList([1.20002])
$$) AS (toFloatList agtype);
+-- large magnitudes must not overflow the conversion (regression: unbounded
+-- sprintf into a fixed stack buffer overflowed for values like 1.0e308)
+SELECT * FROM cypher('expr', $$
+ RETURN toFloatList([1.0e308, -1.0e308])
+$$) AS (toFloatList agtype);
+-- precision must be preserved (regression: "%f" format truncated to 6 digits,
+-- so 0.123456789 came back as 0.123457)
+SELECT * FROM cypher('expr', $$
+ RETURN toFloatList([0.123456789])
+$$) AS (toFloatList agtype);
-- should return null
SELECT * FROM cypher('expr', $$
RETURN toFloatList(['true'])
diff --git a/src/backend/utils/adt/agtype.c b/src/backend/utils/adt/agtype.c
index bf69bf1f..0e1a7963 100644
--- a/src/backend/utils/adt/agtype.c
+++ b/src/backend/utils/adt/agtype.c
@@ -7099,8 +7099,6 @@ Datum age_tofloatlist(PG_FUNCTION_ARGS)
int count;
int i;
bool is_valid = false;
- float8 float_num;
- char buffer[64];
/* check for null */
if (PG_ARGISNULL(0))
@@ -7160,11 +7158,16 @@ Datum age_tofloatlist(PG_FUNCTION_ARGS)
case AGTV_FLOAT:
+ /*
+ * The element is already a float8, so assign it directly. The
+ * previous approach formatted it to a string with sprintf() and
+ * re-parsed it: that both overflowed a fixed 64-byte stack buffer
+ * for large magnitudes (e.g. 1.0e308 needs ~317 chars) and lost
+ * precision, since "%f" truncates to 6 fractional digits. Direct
+ * assignment avoids both problems.
+ */
float_elem.type = AGTV_FLOAT;
- float_num = elem->val.float_value;
- sprintf(buffer, "%f", float_num);
- string = buffer;
- float_elem.val.float_value = float8in_internal_null(string, NULL,
"double precision", string, &is_valid);
+ float_elem.val.float_value = elem->val.float_value;
agis_result.res = push_agtype_value(&agis_result.parse_state,
WAGT_ELEM, &float_elem);
break;
@@ -8146,7 +8149,7 @@ Datum age_tostringlist(PG_FUNCTION_ARGS)
case AGTV_FLOAT:
- sprintf(buffer, "%.*g", DBL_DIG, elem->val.float_value);
+ snprintf(buffer, sizeof(buffer), "%.*g", DBL_DIG,
elem->val.float_value);
string_elem.val.string.val = pstrdup(buffer);
string_elem.val.string.len = strlen(buffer);
@@ -8157,7 +8160,7 @@ Datum age_tostringlist(PG_FUNCTION_ARGS)
case AGTV_INTEGER:
- sprintf(buffer, "%ld", elem->val.int_value);
+ snprintf(buffer, sizeof(buffer), "%ld", elem->val.int_value);
string_elem.val.string.val = pstrdup(buffer);
string_elem.val.string.len = strlen(buffer);
