Re: Avoid overflowed array index (src/backend/utils/activity/pgstat.c)

On Wed, Sep 04, 2024 at 03:14:34PM -0300, Ranier Vilela wrote:
> The commit 7949d95 <http://7949d9594582ab49dee221e1db1aa5401ace49d4>, left
> out an oversight.
>
> The report is:
> CID 1559468: (#1 of 1): Overflowed array index read (INTEGER_OVERFLOW)
>
> I think that Coverity is right.
> In the function *pgstat_read_statsfile* It is necessary to first check
> whether it is the most restrictive case.
>
> Otherwise, if PgStat_Kind is greater than 11, a negative index may occur.

You are missing the fact that there is a call to
pgstat_is_kind_valid() a couple of lines above, meaning that we are
sure that the kind ID we are dealing with is within the range [1,11]
for built-in kinds or [128,256] for the custom kinds, so any ID not
within the first range would just be within the second range.

Speaking of which, I am spotting two possible pointer dereferences
when reading the stats file if we are loading custom stats that do not
exist anymore compared to the moment when they were written, for two
cases:
- Fixed-numbered stats entries.
- Named entries, like replication slot stats, but for the custom case.
It would mean that we'd crash at startup when reading stats depending
on how shared_preload_libraries has changed, which is not the original
intention. The patch includes details to inform what was found
wrong with two new WARNING messages. Will fix in a bit, attaching it
for now.

Kind of interesting that your tool did not spot that, and missed the
two I have noticed considering that we're dealing with the same code
paths. The community coverity did not complain on any of them, AFAIK.
--
Michael