On Fri, Aug 25, 2017 at 8:10 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I think the real problem occurs where we realloc the array bigger.
> tupArrSize needs to be kept to no more than INT_MAX --- and, ideally,
> it should reach that value rather than dying on the iteration after
> it reaches 2^30 (so that we support resultsets as large as we possibly
> can). Without a range-check, it's not very clear what realloc will think
> it's being asked for. Also, on 32-bit machines, we could overflow size_t
> before tupArrSize even gets that big, so a test against
> SIZE_MAX/sizeof(pointer) may be needed as well.
>
> As long as we constrain tupArrSize to be within bounds, we don't
> have to worry about overflow of ntups per se.
I just poked more seriously at this code, and we could use something like that:
@@ -868,6 +868,16 @@ pqAddTuple(PGresult *res, PGresAttValue *tup)
int newSize = (res->tupArrSize > 0) ? res->tupArrSize * 2 : 128;
PGresAttValue **newTuples;
+ if (res->tupArrSize == INT_MAX)
+ return FALSE;
+ if (new_size == INT_MIN)
+ new_size = INT_MAX;
+ if (newSize > SIZE_MAX / sizeof(PGresAttValue *))
+ return FALSE;
Looking at the surroundings, I think that it would be nice to have
pqAddTuple and PQsetvalue set an error message with this patch. The
user can see now that those would only properly report on OOM, but if
we add more types of errors proper error messages would be nice for
users.
--
Michael