From 3f7c5a5cf628290d89503b72e6673e482e9d8f05 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Mon, 17 Feb 2020 14:35:10 +0100 Subject: [PATCH] Semmle: Fix possible infite loops caused by overflow (#3503) Comparison between differently sized integers in loop conditions can cause infinite loops. This can happen when doing something like this: ```c int64 very_big = MAX_INT32 + 1; for (int32 i = 0; i < very_big; i++) { // do something } // never reached because i overflows before it can reach the value of very_big ``` --- .../distributed/connection/connection_configuration.c | 9 ++++----- src/backend/distributed/deparser/citus_ruleutils.c | 8 +++++++- src/backend/distributed/planner/recursive_planning.c | 6 +++++- .../distributed/transaction/citus_dist_stat_activity.c | 2 +- src/backend/distributed/utils/acquire_lock.c | 4 +--- .../distributed/worker/worker_partition_protocol.c | 2 +- 6 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/backend/distributed/connection/connection_configuration.c b/src/backend/distributed/connection/connection_configuration.c index a51bbfcf8..9d1e423e9 100644 --- a/src/backend/distributed/connection/connection_configuration.c +++ b/src/backend/distributed/connection/connection_configuration.c @@ -76,7 +76,7 @@ InitConnParams() void ResetConnParams() { - for (Index paramIdx = 0; paramIdx < ConnParams.size; paramIdx++) + for (Size paramIdx = 0; paramIdx < ConnParams.size; paramIdx++) { free((void *) ConnParams.keywords[paramIdx]); free((void *) ConnParams.values[paramIdx]); @@ -135,7 +135,6 @@ CheckConninfo(const char *conninfo, const char **whitelist, Size whitelistLength, char **errorMsg) { PQconninfoOption *option = NULL; - Index whitelistIdx PG_USED_FOR_ASSERTS_ONLY = 0; char *errorMsgString = NULL; /* @@ -174,7 +173,7 @@ CheckConninfo(const char *conninfo, const char **whitelist, #ifdef USE_ASSERT_CHECKING /* verify that the whitelist is in ascending order */ - for (whitelistIdx = 1; whitelistIdx < whitelistLength; whitelistIdx++) + for (Size whitelistIdx = 1; whitelistIdx < whitelistLength; whitelistIdx++) { const char *prev = whitelist[whitelistIdx - 1]; const char *curr = whitelist[whitelistIdx]; @@ -290,7 +289,7 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values, pg_ltoa(key->port, nodePortString); /* populate node port string with port */ /* first step: copy global parameters to beginning of array */ - for (Index paramIndex = 0; paramIndex < ConnParams.size; paramIndex++) + for (Size paramIndex = 0; paramIndex < ConnParams.size; paramIndex++) { /* copy the keyword&value pointers to the new array */ connKeywords[paramIndex] = ConnParams.keywords[paramIndex]; @@ -328,7 +327,7 @@ GetConnParams(ConnectionHashKey *key, char ***keywords, char ***values, const char * GetConnParam(const char *keyword) { - for (Index i = 0; i < ConnParams.size; i++) + for (Size i = 0; i < ConnParams.size; i++) { if (strcmp(keyword, ConnParams.keywords[i]) == 0) { diff --git a/src/backend/distributed/deparser/citus_ruleutils.c b/src/backend/distributed/deparser/citus_ruleutils.c index f505e2e3b..7a2578341 100644 --- a/src/backend/distributed/deparser/citus_ruleutils.c +++ b/src/backend/distributed/deparser/citus_ruleutils.c @@ -524,7 +524,13 @@ pg_get_tablecolumnoptionsdef_string(Oid tableRelationId) */ TupleDesc tupleDescriptor = RelationGetDescr(relation); - for (AttrNumber attributeIndex = 0; attributeIndex < tupleDescriptor->natts; + if (tupleDescriptor->natts > MaxAttrNumber) + { + ereport(ERROR, (errmsg("bad number of tuple descriptor attributes"))); + } + + for (AttrNumber attributeIndex = 0; + attributeIndex < (AttrNumber) tupleDescriptor->natts; attributeIndex++) { Form_pg_attribute attributeForm = TupleDescAttr(tupleDescriptor, attributeIndex); diff --git a/src/backend/distributed/planner/recursive_planning.c b/src/backend/distributed/planner/recursive_planning.c index 68c465308..4548f5499 100644 --- a/src/backend/distributed/planner/recursive_planning.c +++ b/src/backend/distributed/planner/recursive_planning.c @@ -1397,7 +1397,11 @@ TransformFunctionRTE(RangeTblEntry *rangeTblEntry) * * We will iterate over Tuple Description attributes. i.e (c1 int, c2 text) */ - for (targetColumnIndex = 0; targetColumnIndex < tupleDesc->natts; + if (tupleDesc->natts > MaxAttrNumber) + { + ereport(ERROR, (errmsg("bad number of tuple descriptor attributes"))); + } + for (targetColumnIndex = 0; targetColumnIndex < (AttrNumber) tupleDesc->natts; targetColumnIndex++) { FormData_pg_attribute *attribute = TupleDescAttr(tupleDesc, diff --git a/src/backend/distributed/transaction/citus_dist_stat_activity.c b/src/backend/distributed/transaction/citus_dist_stat_activity.c index 11ad81930..2a7888562 100644 --- a/src/backend/distributed/transaction/citus_dist_stat_activity.c +++ b/src/backend/distributed/transaction/citus_dist_stat_activity.c @@ -611,7 +611,7 @@ LocalNodeCitusDistStat(const char *statQuery, const char *hostname, int port) */ oldContext = MemoryContextSwitchTo(upperContext); - for (uint32 rowIndex = 0; rowIndex < SPI_processed; rowIndex++) + for (uint64 rowIndex = 0; rowIndex < SPI_processed; rowIndex++) { TupleDesc rowDescriptor = SPI_tuptable->tupdesc; diff --git a/src/backend/distributed/utils/acquire_lock.c b/src/backend/distributed/utils/acquire_lock.c index cbd57f5f2..8af1f35fc 100644 --- a/src/backend/distributed/utils/acquire_lock.c +++ b/src/backend/distributed/utils/acquire_lock.c @@ -242,8 +242,6 @@ LockAcquireHelperMain(Datum main_arg) while (ShouldAcquireLock(100)) { - int row = 0; - elog(LOG, "canceling competing backends for backend %d", backendPid); /* @@ -261,7 +259,7 @@ LockAcquireHelperMain(Datum main_arg) if (spiStatus == SPI_OK_SELECT) { - for (row = 0; row < SPI_processed; row++) + for (uint64 row = 0; row < SPI_processed; row++) { bool isnull = false; diff --git a/src/backend/distributed/worker/worker_partition_protocol.c b/src/backend/distributed/worker/worker_partition_protocol.c index 5ebbc245b..2aa4bcf31 100644 --- a/src/backend/distributed/worker/worker_partition_protocol.c +++ b/src/backend/distributed/worker/worker_partition_protocol.c @@ -936,7 +936,7 @@ FilterAndPartitionTable(const char *filterQuery, while (SPI_processed > 0) { - for (int rowIndex = 0; rowIndex < SPI_processed; rowIndex++) + for (uint64 rowIndex = 0; rowIndex < SPI_processed; rowIndex++) { HeapTuple row = SPI_tuptable->vals[rowIndex]; TupleDesc rowDescriptor = SPI_tuptable->tupdesc;