[PATCH][BUG_FIX] Potential null pointer dereferencing.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][BUG_FIX] Potential null pointer dereferencing.

Ranier Vilela
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;
  }
 

nodeUnique.c.patch (502 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Daniel Gustafsson
> On 12 Nov 2019, at 14:07, Ranier Vilela <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

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

Ranier Vilela
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 <[hidden email]>
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 <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

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

Kyotaro Horiguchi-4
At Tue, 12 Nov 2019 14:03:53 +0000, Ranier Vilela <[hidden email]> 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