Thread: [PATCH][BUG_FIX] Potential null pointer dereferencing.

Hi,
ExecClearTuple don't check por NULL pointer arg and according
TupIsNull slot can be NULL.

Can anyone check this buf fix?

--- \dll\postgresql-12.0\a\backend\executor\nodeUnique.c    Mon Sep 30 17:06:55 2019
+++ nodeUnique.c    Tue Nov 12 09:54:34 2019
@@ -74,7 +74,8 @@
         if (TupIsNull(slot))
         {
             /* end of subplan, so we're done */
-            ExecClearTuple(resultTupleSlot);
+            if (!TupIsNull(resultTupleSlot))
+                ExecClearTuple(resultTupleSlot);
             return NULL;
         }

> On 12 Nov 2019, at 14:07, Ranier Vilela <ranier_gyn@hotmail.com> wrote:

> ExecClearTuple don't check por NULL pointer arg and according
> TupIsNull slot can be NULL.

I assume you are referring to the TupIsNull(resultTupleSlot) check a few lines
down in the same loop?  If resultTupleSlot was indeed NULL and not empty, the
subsequent call to ExecCopySlot would be a NULL pointer dereference too.  I
might be missing something obvious, but in which case can resultTupleSlot be
NULL when calling ExecUnique?

cheers ./daniel

Hi,
The condition is :
74.  if (TupIsNull(slot))  is true
85.  if (TupIsNull(resultTupleSlot)) is true too.

If resultTupleSlot is not can be NULL, why test if (TupIsNull(resultTupleSlot))?
Occurring these two conditions ExecClearTuple is called, but, don't check by NULL arg.

There are at least 2 more possible cases, envolving ExecClearTuple:
nodeFunctionscan.c and nodeWindowAgg.c

Best regards,
Ranier Vilela

________________________________________
De: Daniel Gustafsson <daniel@yesql.se>
Enviado: terça-feira, 12 de novembro de 2019 13:43
Para: Ranier Vilela
Cc: PostgreSQL Hackers
Assunto: Re: [PATCH][BUG_FIX] Potential null pointer dereferencing.

> On 12 Nov 2019, at 14:07, Ranier Vilela <ranier_gyn@hotmail.com> wrote:

> ExecClearTuple don't check por NULL pointer arg and according
> TupIsNull slot can be NULL.

I assume you are referring to the TupIsNull(resultTupleSlot) check a few lines
down in the same loop?  If resultTupleSlot was indeed NULL and not empty, the
subsequent call to ExecCopySlot would be a NULL pointer dereference too.  I
might be missing something obvious, but in which case can resultTupleSlot be
NULL when calling ExecUnique?

cheers ./daniel

At Tue, 12 Nov 2019 14:03:53 +0000, Ranier Vilela <ranier_gyn@hotmail.com> wrote in 
> Hi,
> The condition is :
> 74.  if (TupIsNull(slot))  is true
> 85.  if (TupIsNull(resultTupleSlot)) is true too.

See the definition of TupIsNull. It checks the tupleslot at a valid
pointer is EMPTY as well. And node->ps.ps_ResultTupleSlot cannot be
NULL there since ExecInitUnique initializes it. The sequence is
obvious so even Assert is not needed there, I think.

> If resultTupleSlot is not can be NULL, why test if (TupIsNull(resultTupleSlot))?

It checks if there is no stored "previous" tuple, which is used to
detect the next value. If it is EMPTY (not NULL), it is the first
tuple from the outerPlan as described in the comment just above.

> Occurring these two conditions ExecClearTuple is called, but, don't check by NULL arg.
> 
> There are at least 2 more possible cases, envolving ExecClearTuple:
> nodeFunctionscan.c and nodeWindowAgg.c

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center