Re: Possible dereference after null check (src/backend/executor/ExecUtils.c) - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Possible dereference after null check (src/backend/executor/ExecUtils.c)
Date
Msg-id CAEudQAq8cTkxDPuhG22s4XEuqEgP0v1iweue_nitt_4a4OGw7A@mail.gmail.com
Whole thread Raw
In response to Re: Possible dereference after null check (src/backend/executor/ExecUtils.c)  (Ranier Vilela <ranier.vf@gmail.com>)
List pgsql-hackers
Em sex., 12 de fev. de 2021 às 13:11, Ranier Vilela <ranier.vf@gmail.com> escreveu:
Em sex., 12 de fev. de 2021 às 03:28, Kyotaro Horiguchi <horikyota.ntt@gmail.com> escreveu:
At Wed, 10 Feb 2021 19:54:46 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in
> Hi,
>
> Per Coverity.
>
> The functions ExecGetInsertedCols and ExecGetUpdatedCols at ExecUtils.c
> only are safe to call if the variable "ri_RangeTableIndex" is  != 0.
>
> Otherwise a possible Dereference after null check (FORWARD_NULL) can be
> raised.

As it turns out, it's a false positive. And perhaps we don't want to
take action just to satisfy the static code analyzer.


The coment in ExecGetInsertedCols says:

> /*
>  * The columns are stored in the range table entry. If this ResultRelInfo
>  * doesn't have an entry in the range table (i.e. if it represents a
>  * partition routing target), fetch the parent's RTE and map the columns
>  * to the order they are in the partition.
>  */
> if (relinfo->ri_RangeTableIndex != 0)
> {

This means that any one of the two is always usable here.  AFAICS,
actually, ri_RangeTableIndex is non-zero for partitioned (=leaf) and
non-partitoned relations and ri_RootResultRelInfo is non-null for
partitioned (parent or intermediate) relations (since they don't have
a coressponding range table entry).

The only cases where both are 0 and NULL are trigger-use, which is
unrelated to the code path.
This is a case where it would be worth an assertion.
What do you think?
Apparently my efforts with Coverity have been worth it.
And together we are helping to keep Postgres more secure.
Although sometimes it is not recognized for that  [1].

regards,
Ranier Vilela

pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: pg_cryptohash_final possible out-of-bounds access (per Coverity)
Next
From: Peter Eisentraut
Date:
Subject: SSL SNI