On Wed, Jul 17, 2024 at 9:31 PM jian he <jian.universal...@gmail.com> wrote: > > i think "INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{}');" > means to insert one element (size) to a customized lower/upper bounds.
Ah, thank you, I mistakenly understood that as an array with size 2147483647, with the first 2147483646 elements NULL. I've updated the first calculation (upper_bound + 1) to retrun an error saying "array upper bound is too large: %d" when it overflows. This will change some of the existing error messages, but is just as correct and doesn't require us to check the source array. Is there backwards compatibility guarantees on error messages or is that acceptable? For the second calculation ((upper_bound + 1) - lower_bound), I've kept the existing error of "array size exceeds the maximum allowed (%d)". The only way for that to underflow is if the upper bound is very negative and the lower bound is very positive. I'm not entirely sure how to interpret this scenario, but it's consistent with similar scenarios. # INSERT INTO arroverflowtest(i[10:-999999]) VALUES ('{1,2,3}'); ERROR: array size exceeds the maximum allowed (134217727) As a reminder: - 0001 is reviewed. - 0002 is reviewed and a bug fix. - 0003 is currently under review and a bug fix. - 0004 needs a review. Thanks, Joe Koshakow
From 6c47deeb39eb352d7888c9490613dcf18aee198b Mon Sep 17 00:00:00 2001 From: Joseph Koshakow <kosh...@gmail.com> Date: Thu, 13 Jun 2024 22:39:25 -0400 Subject: [PATCH 2/4] Handle overflow in money arithmetic --- src/backend/utils/adt/cash.c | 174 +++++++++++++++------------- src/test/regress/expected/money.out | 106 +++++++++++++++++ src/test/regress/sql/money.sql | 47 ++++++++ 3 files changed, 247 insertions(+), 80 deletions(-) diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index f6f095a57b..52687dbf7b 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -26,6 +26,7 @@ #include "libpq/pqformat.h" #include "utils/builtins.h" #include "utils/cash.h" +#include "utils/float.h" #include "utils/numeric.h" #include "utils/pg_locale.h" @@ -86,6 +87,82 @@ num_word(Cash value) return buf; } /* num_word() */ +static inline Cash +cash_pl_cash(Cash c1, Cash c2) +{ + Cash res; + + if (unlikely(pg_add_s64_overflow(c1, c2, &res))) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("money out of range"))); + + return res; +} + +static inline Cash +cash_mi_cash(Cash c1, Cash c2) +{ + Cash res; + + if (unlikely(pg_sub_s64_overflow(c1, c2, &res))) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("money out of range"))); + + return res; +} + +static inline Cash +cash_mul_float8(Cash c, float8 f) +{ + float8 res = rint(float8_mul((float8) c, f)); + + if (unlikely(isnan(res) || !FLOAT8_FITS_IN_INT64(res))) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("money out of range"))); + + return (Cash) res; +} + +static inline Cash +cash_div_float8(Cash c, float8 f) +{ + float8 res = rint(float8_div((float8) c, f)); + + if (unlikely(isnan(res) || !FLOAT8_FITS_IN_INT64(res))) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("money out of range"))); + + return (Cash) res; +} + +static inline Cash +cash_mul_int64(Cash c, int64 i) +{ + Cash res; + + if (unlikely(pg_mul_s64_overflow(c, i, &res))) + ereport(ERROR, + (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), + errmsg("money out of range"))); + + return res; +} + +static inline Cash +cash_div_int64(Cash c, int64 i) +{ + if (unlikely(i == 0)) + ereport(ERROR, + (errcode(ERRCODE_DIVISION_BY_ZERO), + errmsg("division by zero"))); + + return c / i; +} + /* cash_in() * Convert a string to a cash data type. * Format is [$]###[,]###[.##] @@ -615,11 +692,8 @@ cash_pl(PG_FUNCTION_ARGS) { Cash c1 = PG_GETARG_CASH(0); Cash c2 = PG_GETARG_CASH(1); - Cash result; - - result = c1 + c2; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_pl_cash(c1, c2)); } @@ -631,11 +705,8 @@ cash_mi(PG_FUNCTION_ARGS) { Cash c1 = PG_GETARG_CASH(0); Cash c2 = PG_GETARG_CASH(1); - Cash result; - - result = c1 - c2; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mi_cash(c1, c2)); } @@ -667,10 +738,8 @@ cash_mul_flt8(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); float8 f = PG_GETARG_FLOAT8(1); - Cash result; - result = rint(c * f); - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_float8(c, f)); } @@ -682,10 +751,8 @@ flt8_mul_cash(PG_FUNCTION_ARGS) { float8 f = PG_GETARG_FLOAT8(0); Cash c = PG_GETARG_CASH(1); - Cash result; - result = rint(f * c); - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_float8(c, f)); } @@ -697,15 +764,8 @@ cash_div_flt8(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); float8 f = PG_GETARG_FLOAT8(1); - Cash result; - if (f == 0.0) - ereport(ERROR, - (errcode(ERRCODE_DIVISION_BY_ZERO), - errmsg("division by zero"))); - - result = rint(c / f); - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_div_float8(c, f)); } @@ -717,10 +777,8 @@ cash_mul_flt4(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); float4 f = PG_GETARG_FLOAT4(1); - Cash result; - result = rint(c * (float8) f); - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_float8(c, (float8) f)); } @@ -732,10 +790,8 @@ flt4_mul_cash(PG_FUNCTION_ARGS) { float4 f = PG_GETARG_FLOAT4(0); Cash c = PG_GETARG_CASH(1); - Cash result; - result = rint((float8) f * c); - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_float8(c, (float8) f)); } @@ -748,15 +804,8 @@ cash_div_flt4(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); float4 f = PG_GETARG_FLOAT4(1); - Cash result; - - if (f == 0.0) - ereport(ERROR, - (errcode(ERRCODE_DIVISION_BY_ZERO), - errmsg("division by zero"))); - result = rint(c / (float8) f); - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_div_float8(c, (float8) f)); } @@ -768,10 +817,8 @@ cash_mul_int8(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); int64 i = PG_GETARG_INT64(1); - Cash result; - result = c * i; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_int64(c, i)); } @@ -783,10 +830,8 @@ int8_mul_cash(PG_FUNCTION_ARGS) { int64 i = PG_GETARG_INT64(0); Cash c = PG_GETARG_CASH(1); - Cash result; - result = i * c; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_int64(c, i)); } /* cash_div_int8() @@ -797,16 +842,8 @@ cash_div_int8(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); int64 i = PG_GETARG_INT64(1); - Cash result; - - if (i == 0) - ereport(ERROR, - (errcode(ERRCODE_DIVISION_BY_ZERO), - errmsg("division by zero"))); - result = c / i; - - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_div_int64(c, i)); } @@ -818,10 +855,8 @@ cash_mul_int4(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); int32 i = PG_GETARG_INT32(1); - Cash result; - result = c * i; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_int64(c, (int64) i)); } @@ -833,10 +868,8 @@ int4_mul_cash(PG_FUNCTION_ARGS) { int32 i = PG_GETARG_INT32(0); Cash c = PG_GETARG_CASH(1); - Cash result; - result = i * c; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_int64(c, (int64) i)); } @@ -849,16 +882,8 @@ cash_div_int4(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); int32 i = PG_GETARG_INT32(1); - Cash result; - - if (i == 0) - ereport(ERROR, - (errcode(ERRCODE_DIVISION_BY_ZERO), - errmsg("division by zero"))); - - result = c / i; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_div_int64(c, (int64) i)); } @@ -870,10 +895,8 @@ cash_mul_int2(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); int16 s = PG_GETARG_INT16(1); - Cash result; - result = c * s; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_int64(c, (int64) s)); } /* int2_mul_cash() @@ -884,10 +907,8 @@ int2_mul_cash(PG_FUNCTION_ARGS) { int16 s = PG_GETARG_INT16(0); Cash c = PG_GETARG_CASH(1); - Cash result; - result = s * c; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_mul_int64(c, (int64) s)); } /* cash_div_int2() @@ -899,15 +920,8 @@ cash_div_int2(PG_FUNCTION_ARGS) { Cash c = PG_GETARG_CASH(0); int16 s = PG_GETARG_INT16(1); - Cash result; - if (s == 0) - ereport(ERROR, - (errcode(ERRCODE_DIVISION_BY_ZERO), - errmsg("division by zero"))); - - result = c / s; - PG_RETURN_CASH(result); + PG_RETURN_CASH(cash_div_int64(c, (int64) s)); } /* cashlarger() diff --git a/src/test/regress/expected/money.out b/src/test/regress/expected/money.out index 7fd4e31804..890ce1da63 100644 --- a/src/test/regress/expected/money.out +++ b/src/test/regress/expected/money.out @@ -528,3 +528,109 @@ SELECT '-92233720368547758.08'::money::numeric; -92233720368547758.08 (1 row) +-- Test overflow checks +SELECT '92233720368547758.07'::money + '0.01'::money; +ERROR: money out of range +SELECT '-92233720368547758.08'::money - '0.01'::money; +ERROR: money out of range +SELECT '92233720368547758.07'::money * 2::int8; +ERROR: money out of range +SELECT '-92233720368547758.08'::money * 2::int8; +ERROR: money out of range +SELECT 2::int8 * '92233720368547758.07'::money ; +ERROR: money out of range +SELECT 2::int8 * '-92233720368547758.08'::money; +ERROR: money out of range +SELECT '92233720368547758.07'::money * 2::int4; +ERROR: money out of range +SELECT '-92233720368547758.08'::money * 2::int4; +ERROR: money out of range +SELECT 2::int4 * '92233720368547758.07'::money ; +ERROR: money out of range +SELECT 2::int4 * '-92233720368547758.08'::money; +ERROR: money out of range +SELECT '92233720368547758.07'::money * 2::int2; +ERROR: money out of range +SELECT '-92233720368547758.08'::money * 2::int2; +ERROR: money out of range +SELECT 2::int2 * '92233720368547758.07'::money ; +ERROR: money out of range +SELECT 2::int2 * '-92233720368547758.08'::money; +ERROR: money out of range +SELECT '92233720368547758.07'::money * 2::float8; +ERROR: money out of range +SELECT '-92233720368547758.08'::money * 2::float8; +ERROR: money out of range +SELECT 2::float8 * '92233720368547758.07'::money ; +ERROR: money out of range +SELECT 2::float8 * '-92233720368547758.08'::money; +ERROR: money out of range +SELECT '92233720368547758.07'::money * 2::float4; +ERROR: money out of range +SELECT '-92233720368547758.08'::money * 2::float4; +ERROR: money out of range +SELECT 2::float4 * '92233720368547758.07'::money ; +ERROR: money out of range +SELECT 2::float4 * '-92233720368547758.08'::money; +ERROR: money out of range +SELECT '1'::money / 1.175494e-38::float8; +ERROR: money out of range +SELECT '-1'::money / 1.175494e-38::float8; +ERROR: money out of range +SELECT '1'::money / 1.175494e-38::float4; +ERROR: money out of range +SELECT '-1'::money / 1.175494e-38::float4; +ERROR: money out of range +-- Test invalid multipliers/divisors +SELECT '42'::money * 'inf'::float8; +ERROR: money out of range +SELECT '42'::money * '-inf'::float8; +ERROR: money out of range +SELECT '42'::money * 'nan'::float8; +ERROR: money out of range +SELECT 'inf'::float8 * '42'::money; +ERROR: money out of range +SELECT '-inf'::float8 * '42'::money; +ERROR: money out of range +SELECT 'nan'::float8 * '42'::money; +ERROR: money out of range +SELECT '42'::money / 'inf'::float8; + ?column? +---------- + $0.00 +(1 row) + +SELECT '42'::money / '-inf'::float8; + ?column? +---------- + $0.00 +(1 row) + +SELECT '42'::money / 'nan'::float8; +ERROR: money out of range +SELECT '42'::money * 'inf'::float4; +ERROR: money out of range +SELECT '42'::money * '-inf'::float4; +ERROR: money out of range +SELECT '42'::money * 'nan'::float4; +ERROR: money out of range +SELECT 'inf'::float4 * '42'::money; +ERROR: money out of range +SELECT '-inf'::float4 * '42'::money; +ERROR: money out of range +SELECT 'nan'::float4 * '42'::money; +ERROR: money out of range +SELECT '42'::money / 'inf'::float4; + ?column? +---------- + $0.00 +(1 row) + +SELECT '42'::money / '-inf'::float4; + ?column? +---------- + $0.00 +(1 row) + +SELECT '42'::money / 'nan'::float4; +ERROR: money out of range diff --git a/src/test/regress/sql/money.sql b/src/test/regress/sql/money.sql index 81c92dd960..4422d2816b 100644 --- a/src/test/regress/sql/money.sql +++ b/src/test/regress/sql/money.sql @@ -135,3 +135,50 @@ SELECT '12345678901234567'::money::numeric; SELECT '-12345678901234567'::money::numeric; SELECT '92233720368547758.07'::money::numeric; SELECT '-92233720368547758.08'::money::numeric; + +-- Test overflow checks +SELECT '92233720368547758.07'::money + '0.01'::money; +SELECT '-92233720368547758.08'::money - '0.01'::money; +SELECT '92233720368547758.07'::money * 2::int8; +SELECT '-92233720368547758.08'::money * 2::int8; +SELECT 2::int8 * '92233720368547758.07'::money ; +SELECT 2::int8 * '-92233720368547758.08'::money; +SELECT '92233720368547758.07'::money * 2::int4; +SELECT '-92233720368547758.08'::money * 2::int4; +SELECT 2::int4 * '92233720368547758.07'::money ; +SELECT 2::int4 * '-92233720368547758.08'::money; +SELECT '92233720368547758.07'::money * 2::int2; +SELECT '-92233720368547758.08'::money * 2::int2; +SELECT 2::int2 * '92233720368547758.07'::money ; +SELECT 2::int2 * '-92233720368547758.08'::money; +SELECT '92233720368547758.07'::money * 2::float8; +SELECT '-92233720368547758.08'::money * 2::float8; +SELECT 2::float8 * '92233720368547758.07'::money ; +SELECT 2::float8 * '-92233720368547758.08'::money; +SELECT '92233720368547758.07'::money * 2::float4; +SELECT '-92233720368547758.08'::money * 2::float4; +SELECT 2::float4 * '92233720368547758.07'::money ; +SELECT 2::float4 * '-92233720368547758.08'::money; +SELECT '1'::money / 1.175494e-38::float8; +SELECT '-1'::money / 1.175494e-38::float8; +SELECT '1'::money / 1.175494e-38::float4; +SELECT '-1'::money / 1.175494e-38::float4; +-- Test invalid multipliers/divisors +SELECT '42'::money * 'inf'::float8; +SELECT '42'::money * '-inf'::float8; +SELECT '42'::money * 'nan'::float8; +SELECT 'inf'::float8 * '42'::money; +SELECT '-inf'::float8 * '42'::money; +SELECT 'nan'::float8 * '42'::money; +SELECT '42'::money / 'inf'::float8; +SELECT '42'::money / '-inf'::float8; +SELECT '42'::money / 'nan'::float8; +SELECT '42'::money * 'inf'::float4; +SELECT '42'::money * '-inf'::float4; +SELECT '42'::money * 'nan'::float4; +SELECT 'inf'::float4 * '42'::money; +SELECT '-inf'::float4 * '42'::money; +SELECT 'nan'::float4 * '42'::money; +SELECT '42'::money / 'inf'::float4; +SELECT '42'::money / '-inf'::float4; +SELECT '42'::money / 'nan'::float4; -- 2.34.1
From c125f8a2cd5e3e5751087979df29868437d11ab0 Mon Sep 17 00:00:00 2001 From: Joseph Koshakow <kosh...@gmail.com> Date: Sat, 6 Jul 2024 15:41:09 -0400 Subject: [PATCH 4/4] Remove dependence on integer wrapping for jsonb This commit updates various jsonb operators and functions to no longer rely on integer wrapping for correctness. Not all compilers support -fwrapv, so it's best not to rely on it. --- src/backend/utils/adt/jsonfuncs.c | 4 ++-- src/test/regress/expected/jsonb.out | 12 ++++++++++++ src/test/regress/sql/jsonb.sql | 2 ++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 48c3f88140..8783c57303 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -946,7 +946,7 @@ jsonb_array_element(PG_FUNCTION_ARGS) { uint32 nelements = JB_ROOT_COUNT(jb); - if (-element > nelements) + if (element == PG_INT32_MIN || -element > nelements) PG_RETURN_NULL(); else element += nelements; @@ -5425,7 +5425,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls, if (idx < 0) { - if (-idx > nelems) + if (idx == INT_MIN || -idx > nelems) { /* * If asked to keep elements position consistent, it's not allowed diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index e66d760189..a9d93052fc 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -680,6 +680,18 @@ select '"foo"'::jsonb -> 'z'; (1 row) +select '[]'::jsonb -> -2147483648; + ?column? +---------- + +(1 row) + +select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}'); + jsonb_delete_path +------------------- + {"a": []} +(1 row) + select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text; ?column? ---------- diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index 97bc2242a1..6a18577ead 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -204,6 +204,8 @@ select '[{"b": "c"}, {"b": "cc"}]'::jsonb -> 'z'; select '{"a": "c", "b": null}'::jsonb -> 'b'; select '"foo"'::jsonb -> 1; select '"foo"'::jsonb -> 'z'; +select '[]'::jsonb -> -2147483648; +select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}'); select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text; select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::int; -- 2.34.1
From dedcc3e925f89dfc0049d47253a6dfbaafb0409f Mon Sep 17 00:00:00 2001 From: Joseph Koshakow <kosh...@gmail.com> Date: Sat, 8 Jun 2024 22:16:46 -0400 Subject: [PATCH 1/4] Remove dependence on integer wrapping This commit updates various parts of the code to no longer rely on integer wrapping for correctness. Not all compilers support -fwrapv, so it's best not to rely on it. --- src/backend/utils/adt/cash.c | 7 +- src/backend/utils/adt/numeric.c | 5 +- src/backend/utils/adt/numutils.c | 34 ++++--- src/backend/utils/adt/timestamp.c | 28 +----- src/include/common/int.h | 105 +++++++++++++++++++++ src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +-- src/test/regress/expected/timestamp.out | 13 +++ src/test/regress/expected/timestamptz.out | 13 +++ src/test/regress/sql/timestamp.sql | 4 + src/test/regress/sql/timestamptz.sql | 4 + 10 files changed, 169 insertions(+), 55 deletions(-) diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index 32fbad2f57..f6f095a57b 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -352,8 +352,11 @@ cash_out(PG_FUNCTION_ARGS) if (value < 0) { - /* make the amount positive for digit-reconstruction loop */ - value = -value; + /* + * make the amount positive for digit-reconstruction loop, we can + * leave INT64_MIN unchanged + */ + pg_neg_s64_overflow(value, &value); /* set up formatting data */ signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-"; sign_posn = lconvert->n_sign_posn; diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index d0f0923710..38965b4023 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -8114,15 +8114,14 @@ int64_to_numericvar(int64 val, NumericVar *var) /* int64 can require at most 19 decimal digits; add one for safety */ alloc_var(var, 20 / DEC_DIGITS); + uval = pg_abs_s64(val); if (val < 0) { var->sign = NUMERIC_NEG; - uval = -val; } else { var->sign = NUMERIC_POS; - uval = val; } var->dscale = 0; if (val == 0) @@ -11443,7 +11442,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale, * Now we can proceed with the multiplications. */ neg = (exp < 0); - mask = abs(exp); + mask = pg_abs_s32(exp); init_var(&base_prod); set_var_from_var(base, &base_prod); diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c index adc1e8a4cb..a3d7d6bf01 100644 --- a/src/backend/utils/adt/numutils.c +++ b/src/backend/utils/adt/numutils.c @@ -18,6 +18,7 @@ #include <limits.h> #include <ctype.h> +#include "common/int.h" #include "port/pg_bitutils.h" #include "utils/builtins.h" @@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext) uint16 tmp = 0; bool neg = false; unsigned char digit; + int16 result; /* * The majority of cases are likely to be base-10 digits without any @@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext) if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)) + if (pg_neg_u16_overflow(tmp, &result)) goto out_of_range; - return -((int16) tmp); + return result; } if (unlikely(tmp > PG_INT16_MAX)) @@ -333,10 +334,9 @@ slow: if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1) + if (pg_neg_u16_overflow(tmp, &result)) goto out_of_range; - return -((int16) tmp); + return result; } if (tmp > PG_INT16_MAX) @@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext) uint32 tmp = 0; bool neg = false; unsigned char digit; + int32 result; /* * The majority of cases are likely to be base-10 digits without any @@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext) if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)) + if (pg_neg_u32_overflow(tmp, &result)) goto out_of_range; - return -((int32) tmp); + return result; } if (unlikely(tmp > PG_INT32_MAX)) @@ -595,10 +595,9 @@ slow: if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1) + if (pg_neg_u32_overflow(tmp, &result)) goto out_of_range; - return -((int32) tmp); + return result; } if (tmp > PG_INT32_MAX) @@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext) uint64 tmp = 0; bool neg = false; unsigned char digit; + int64 result; /* * The majority of cases are likely to be base-10 digits without any @@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext) if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)) + if (pg_neg_u64_overflow(tmp, &result)) goto out_of_range; - return -((int64) tmp); + return result; } if (unlikely(tmp > PG_INT64_MAX)) @@ -857,10 +856,9 @@ slow: if (neg) { - /* check the negative equivalent will fit without overflowing */ - if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1) + if (pg_neg_u64_overflow(tmp, &result)) goto out_of_range; - return -((int64) tmp); + return result; } if (tmp > PG_INT64_MAX) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 69fe7860ed..c76793f72d 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day, time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE) * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC); - result = date * USECS_PER_DAY + time; - /* check for major overflow */ - if ((result - time) / USECS_PER_DAY != date) - ereport(ERROR, - (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), - errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g", - year, month, day, - hour, min, sec))); - - /* check for just-barely overflow (okay except time-of-day wraps) */ - /* caution: we want to allow 1999-12-31 24:00:00 */ - if ((result < 0 && date > 0) || - (result > 0 && date < -1)) + if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) || + pg_add_s64_overflow(result, time, &result)) ereport(ERROR, (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g", @@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result) date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE; time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec); - *result = date * USECS_PER_DAY + time; - /* check for major overflow */ - if ((*result - time) / USECS_PER_DAY != date) - { - *result = 0; /* keep compiler quiet */ - return -1; - } - /* check for just-barely overflow (okay except time-of-day wraps) */ - /* caution: we want to allow 1999-12-31 24:00:00 */ - if ((*result < 0 && date > 0) || - (*result > 0 && date < -1)) + if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) || + pg_add_s64_overflow(*result, time, result)) { *result = 0; /* keep compiler quiet */ return -1; diff --git a/src/include/common/int.h b/src/include/common/int.h index 7fc046e78a..3065d586d4 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -154,6 +154,23 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline uint32 +pg_abs_s32(int32 a) +{ + if (unlikely(a == PG_INT32_MIN)) + { + return ((uint32) PG_INT32_MAX) + 1; + } + else if (a < 0) + { + return (uint32) -a; + } + else + { + return (uint32) a; + } +} + /* * INT64 */ @@ -258,6 +275,37 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ + if (unlikely(a == PG_INT64_MIN)) + { + return true; + } + else + { + *result = -a; + return false; + } +} + +static inline uint64 +pg_abs_s64(int64 a) +{ + if (unlikely(a == PG_INT64_MIN)) + { + return ((uint64) PG_INT64_MAX) + 1; + } + else if (a < 0) + { + return (uint64) -a; + } + else + { + return (uint64) a; + } +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ @@ -318,6 +366,25 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result) #endif } +static inline bool +pg_neg_u16_overflow(uint16 a, int16 *result) +{ + if (unlikely(a > ((uint16) PG_INT16_MAX) + 1)) + { + return true; + } + else if (unlikely(a == ((uint16) PG_INT16_MAX) + 1)) + { + *result = PG_INT16_MIN; + return false; + } + else + { + *result = -((int16) a); + return false; + } +} + /* * INT32 */ @@ -373,6 +440,25 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result) #endif } +static inline bool +pg_neg_u32_overflow(uint32 a, int32 *result) +{ + if (unlikely(a > ((uint32) PG_INT32_MAX) + 1)) + { + return true; + } + else if (unlikely(a == ((uint32) PG_INT32_MAX) + 1)) + { + *result = PG_INT32_MIN; + return false; + } + else + { + *result = -((int32) a); + return false; + } +} + /* * UINT64 */ @@ -438,6 +524,25 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result) #endif } +static inline bool +pg_neg_u64_overflow(uint64 a, int64 *result) +{ + if (unlikely(a > ((uint64) PG_INT64_MAX) + 1)) + { + return true; + } + else if (unlikely(a == ((uint64) PG_INT64_MAX) + 1)) + { + *result = PG_INT64_MIN; + return false; + } + else + { + *result = -((int64) a); + return false; + } +} + /*------------------------------------------------------------------------ * * Comparison routines for integer types. diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c index f1b143fbd2..93d4cc323d 100644 --- a/src/interfaces/ecpg/pgtypeslib/timestamp.c +++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c @@ -11,6 +11,7 @@ #error -ffast-math is known to break this code #endif +#include "common/int.h" #include "dt.h" #include "pgtypes_date.h" #include "pgtypes_timestamp.h" @@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result) dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1); time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec); - *result = (dDate * USECS_PER_DAY) + time; - /* check for major overflow */ - if ((*result - time) / USECS_PER_DAY != dDate) - return -1; - /* check for just-barely overflow (okay except time-of-day wraps) */ - /* caution: we want to allow 1999-12-31 24:00:00 */ - if ((*result < 0 && dDate > 0) || - (*result > 0 && dDate < -1)) + if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) || + pg_add_s64_overflow(*result, time, result)) return -1; if (tzp != NULL) *result = dt2local(*result, -(*tzp)); diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out index cf337da517..e287260051 100644 --- a/src/test/regress/expected/timestamp.out +++ b/src/test/regress/expected/timestamp.out @@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity'); select age(timestamp '-infinity', timestamp '-infinity'); ERROR: interval out of range +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamp '1999-12-31 24:00:00'; + timestamp +-------------------------- + Sat Jan 01 00:00:00 2000 +(1 row) + +select make_timestamp(1999, 12, 31, 24, 0, 0); + make_timestamp +-------------------------- + Sat Jan 01 00:00:00 2000 +(1 row) + diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out index bfb3825ff6..d01d174983 100644 --- a/src/test/regress/expected/timestamptz.out +++ b/src/test/regress/expected/timestamptz.out @@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity'); SELECT age(timestamptz '-infinity', timestamptz '-infinity'); ERROR: interval out of range +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamptz '1999-12-31 24:00:00'; + timestamptz +------------------------------ + Sat Jan 01 00:00:00 2000 PST +(1 row) + +select make_timestamptz(1999, 12, 31, 24, 0, 0); + make_timestamptz +------------------------------ + Sat Jan 01 00:00:00 2000 PST +(1 row) + diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql index 820ef7752a..748469576d 100644 --- a/src/test/regress/sql/timestamp.sql +++ b/src/test/regress/sql/timestamp.sql @@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity'); select age(timestamp 'infinity', timestamp '-infinity'); select age(timestamp '-infinity', timestamp 'infinity'); select age(timestamp '-infinity', timestamp '-infinity'); + +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamp '1999-12-31 24:00:00'; +select make_timestamp(1999, 12, 31, 24, 0, 0); diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql index ccfd90d646..c71d5489b4 100644 --- a/src/test/regress/sql/timestamptz.sql +++ b/src/test/regress/sql/timestamptz.sql @@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity'); SELECT age(timestamptz 'infinity', timestamptz '-infinity'); SELECT age(timestamptz '-infinity', timestamptz 'infinity'); SELECT age(timestamptz '-infinity', timestamptz '-infinity'); + +-- test timestamp near POSTGRES_EPOCH_JDATE +select timestamptz '1999-12-31 24:00:00'; +select make_timestamptz(1999, 12, 31, 24, 0, 0); -- 2.34.1
From 9bdb0ffd636a366d070c0366e5c94447bb18825e Mon Sep 17 00:00:00 2001 From: Joseph Koshakow <kosh...@gmail.com> Date: Sat, 6 Jul 2024 14:35:00 -0400 Subject: [PATCH 3/4] Remove overflow from array_set_slice This commit removes an overflow from array_set_slice that allows seting absurd slice ranges. --- src/backend/utils/adt/arrayfuncs.c | 12 +++++++++++- src/test/regress/expected/arrays.out | 12 ++++++++++++ src/test/regress/sql/arrays.sql | 8 ++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index d6641b570d..70d9fd119d 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -2887,7 +2887,17 @@ array_set_slice(Datum arraydatum, errdetail("When assigning to a slice of an empty array value," " slice boundaries must be fully specified."))); - dim[i] = 1 + upperIndx[i] - lowerIndx[i]; + /* dim[i] = 1 + upperIndx[i] - lowerIndx[i]; */ + if (pg_add_s32_overflow(1, upperIndx[i], &dim[i])) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("array upper bound is too large: %d", + upperIndx[i]))); + if (pg_sub_s32_overflow(dim[i], lowerIndx[i], &dim[i])) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("array size exceeds the maximum allowed (%d)", + (int) MaxArraySize))); lb[i] = lowerIndx[i]; } diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out index 23404982f7..3d9402dee3 100644 --- a/src/test/regress/expected/arrays.out +++ b/src/test/regress/expected/arrays.out @@ -2699,3 +2699,15 @@ SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail ERROR: sample size must be between 0 and 6 SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail ERROR: sample size must be between 0 and 6 +-- Test for overflow in array slicing +CREATE temp table arroverflowtest (i int[]); +INSERT INTO arroverflowtest(i[-2147483648:2147483647]) VALUES ('{}'); +ERROR: array upper bound is too large: 2147483647 +INSERT INTO arroverflowtest(i[1:2147483647]) VALUES ('{}'); +ERROR: array upper bound is too large: 2147483647 +INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{}'); +ERROR: array upper bound is too large: 2147483647 +INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{1}'); +ERROR: array upper bound is too large: 2147483647 +INSERT INTO arroverflowtest(i[-2147483648:10]) VALUES ('{}'); +ERROR: array size exceeds the maximum allowed (134217727) diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql index 50aa539fdc..e657e80ba2 100644 --- a/src/test/regress/sql/arrays.sql +++ b/src/test/regress/sql/arrays.sql @@ -825,3 +825,11 @@ SELECT array_dims(array_sample('[-1:2][2:3]={{1,2},{3,NULL},{5,6},{7,8}}'::int[] SELECT array_dims(array_sample('{{{1,2},{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2)); SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail + +-- Test for overflow in array slicing +CREATE temp table arroverflowtest (i int[]); +INSERT INTO arroverflowtest(i[-2147483648:2147483647]) VALUES ('{}'); +INSERT INTO arroverflowtest(i[1:2147483647]) VALUES ('{}'); +INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{}'); +INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{1}'); +INSERT INTO arroverflowtest(i[-2147483648:10]) VALUES ('{}'); -- 2.34.1