Re: pgsql: PL/Python: Fix potential NULL pointer dereference

[Peter Eisentraut]

On 12/5/17 14:45, Tom Lane wrote:
> On the whole it seems like it might be better to dodge this whole business
> of changing "result" inside the TRY.  You could do that if you did
> something like
> 
>                 result->rows = PyList_New(rows);
> -               if (!result->rows)
> -               {
> -                   Py_DECREF(result);
> -                   result = NULL;
> -               }
> -               else
> +               if (result->rows)
>                 {
>                     PLy_input_setup_tuple(&ininfo, tuptable->tupdesc,
> 
> and then add after the PG_END_TRY
> 
>                 if (!result->rows)
>                 {
>                     Py_DECREF(result);
>                     result = NULL;
>                 }

Yeah, that looks much better.  Next try attached.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

From aa2f7ca289b3475f1680b181b08be84bda5d74bc Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 5 Dec 2017 14:14:55 -0500
Subject: [PATCH v2] PL/Python: Fix potential NULL pointer dereference

After d0aa965c0a0ac2ff7906ae1b1dad50a7952efa56, one error path in
PLy_spi_execute_fetch_result() could result in the variable "result"
being dereferenced after being set to NULL.  Put a conditional around
that to fix that.

Also add another SPI_freetuptable() call so that that is cleared in all
error paths.

discovered by John Naylor <jcnay...@gmail.com> via scan-build
---
 src/pl/plpython/plpy_spi.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c
index ade27f3924..0c623a9458 100644
--- a/src/pl/plpython/plpy_spi.c
+++ b/src/pl/plpython/plpy_spi.c
@@ -361,7 +361,10 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, 
uint64 rows, int status)
 
        result = (PLyResultObject *) PLy_result_new();
        if (!result)
+       {
+               SPI_freetuptable(tuptable);
                return NULL;
+       }
        Py_DECREF(result->status);
        result->status = PyInt_FromLong(status);
 
@@ -411,12 +414,7 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, 
uint64 rows, int status)
 
                                Py_DECREF(result->rows);
                                result->rows = PyList_New(rows);
-                               if (!result->rows)
-                               {
-                                       Py_DECREF(result);
-                                       result = NULL;
-                               }
-                               else
+                               if (result->rows)
                                {
                                        PLy_input_setup_tuple(&ininfo, 
tuptable->tupdesc,
                                                                                
  exec_ctx->curr_proc);
@@ -455,6 +453,13 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, 
uint64 rows, int status)
 
                MemoryContextDelete(cxt);
                SPI_freetuptable(tuptable);
+
+               /* in case PyList_New() failed above */
+               if (!result->rows)
+               {
+                       Py_DECREF(result);
+                       result = NULL;
+               }
        }
 
        return (PyObject *) result;

base-commit: d329dc2ea4bfac84ec60ba14b96561a7508bb37b
-- 
2.15.1