Re: Protect against possible memory corruption (src/backend/access/nbtree/nbtxlog.c)

On 11/07/2021 22:51, Ranier Vilela wrote:
> Hi,
>
> While analyzing a possible use of an uninitialized variable, I checked that
> *_bt_restore_page* can lead to memory corruption,
> by not checking the maximum limit of array items which is
> MaxIndexTuplesPerPage.

> + /* Protect against corrupted recovery file */
> + nitems = (len / sizeof(IndexTupleData));
> + if (nitems < 0 || nitems > MaxIndexTuplesPerPage)
> + elog(PANIC, "_bt_restore_page: cannot restore %d items to page", nitems);
> +

That's not right. You don't get the number of items by dividing like
that. 'len' includes the tuple data as well, not just the IndexTupleData
header.

> @@ -73,12 +79,9 @@ _bt_restore_page(Page page, char *from, int len)
> nitems = i;
>
> for (i = nitems - 1; i >= 0; i--)
> - {
> if (PageAddItem(page, items[i], itemsizes[i], nitems - i,
> false, false) == InvalidOffsetNumber)
> elog(PANIC, "_bt_restore_page: cannot add item to page");
> - from += itemsz;
> - }
> }

I agree with this change (except that I would leave the braces in
place). The 'from' that's calculated here is plain wrong; oversight in
commit 7e30c186da. Fortunately it's not used, so it can just be removed.

- Heikki