From f7ea240aa7491b6ed2985bb50888bd432f3341df Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 3 Mar 2022 20:03:47 -0500 Subject: [PATCH] Tighten overflow checks in tidin(). This code seems to have been written on the assumption that "unsigned long" is 32 bits; or at any rate it ignored the possibility of conversion overflow. Rewrite, borrowing some logic from oidin(). Discussion: https://postgr.es/m/3441768.1646343914@sss.pgh.pa.us --- src/backend/utils/adt/tid.c | 28 +++++++++++++++++++++------- src/test/regress/expected/tid.out | 19 +++++++++++++++++++ src/test/regress/sql/tid.sql | 12 ++++++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c index dcc1620afb..83ac589f95 100644 --- a/src/backend/utils/adt/tid.c +++ b/src/backend/utils/adt/tid.c @@ -64,10 +64,10 @@ tidin(PG_FUNCTION_ARGS) BlockNumber blockNumber; OffsetNumber offsetNumber; char *badp; - int hold_offset; + unsigned long cvt; for (i = 0, p = str; *p && i < NTIDARGS && *p != RDELIM; p++) - if (*p == DELIM || (*p == LDELIM && !i)) + if (*p == DELIM || (*p == LDELIM && i == 0)) coord[i++] = p + 1; if (i < NTIDARGS) @@ -77,22 +77,36 @@ tidin(PG_FUNCTION_ARGS) "tid", str))); errno = 0; - blockNumber = strtoul(coord[0], &badp, 10); + cvt = strtoul(coord[0], &badp, 10); if (errno || *badp != DELIM) ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", "tid", str))); + blockNumber = (BlockNumber) cvt; - hold_offset = strtol(coord[1], &badp, 10); - if (errno || *badp != RDELIM || - hold_offset > USHRT_MAX || hold_offset < 0) + /* + * Cope with possibility that unsigned long is wider than BlockNumber, in + * which case strtoul will not raise an error for some values that are out + * of the range of BlockNumber. (See similar code in oidin().) + */ +#if SIZEOF_LONG > 4 + if (cvt != (unsigned long) blockNumber && + cvt != (unsigned long) ((int32) blockNumber)) ereport(ERROR, (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type %s: \"%s\"", "tid", str))); +#endif - offsetNumber = hold_offset; + cvt = strtoul(coord[1], &badp, 10); + if (errno || *badp != RDELIM || + cvt > USHRT_MAX) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), + errmsg("invalid input syntax for type %s: \"%s\"", + "tid", str))); + offsetNumber = (OffsetNumber) cvt; result = (ItemPointer) palloc(sizeof(ItemPointerData)); diff --git a/src/test/regress/expected/tid.out b/src/test/regress/expected/tid.out index 8da1a45576..7d8957bd6f 100644 --- a/src/test/regress/expected/tid.out +++ b/src/test/regress/expected/tid.out @@ -1,3 +1,22 @@ +-- basic tests for the TID data type +SELECT + '(0,0)'::tid as tid00, + '(0,1)'::tid as tid01, + '(-1,0)'::tid as tidm10, + '(4294967295,65535)'::tid as tidmax; + tid00 | tid01 | tidm10 | tidmax +-------+-------+----------------+-------------------- + (0,0) | (0,1) | (4294967295,0) | (4294967295,65535) +(1 row) + +SELECT '(4294967296,1)'::tid; -- error +ERROR: invalid input syntax for type tid: "(4294967296,1)" +LINE 1: SELECT '(4294967296,1)'::tid; + ^ +SELECT '(1,65536)'::tid; -- error +ERROR: invalid input syntax for type tid: "(1,65536)" +LINE 1: SELECT '(1,65536)'::tid; + ^ -- tests for functions related to TID handling CREATE TABLE tid_tab (a int); -- min() and max() for TIDs diff --git a/src/test/regress/sql/tid.sql b/src/test/regress/sql/tid.sql index 34546a3cb7..990d314a5f 100644 --- a/src/test/regress/sql/tid.sql +++ b/src/test/regress/sql/tid.sql @@ -1,3 +1,15 @@ +-- basic tests for the TID data type + +SELECT + '(0,0)'::tid as tid00, + '(0,1)'::tid as tid01, + '(-1,0)'::tid as tidm10, + '(4294967295,65535)'::tid as tidmax; + +SELECT '(4294967296,1)'::tid; -- error +SELECT '(1,65536)'::tid; -- error + + -- tests for functions related to TID handling CREATE TABLE tid_tab (a int);