]> granicus.if.org Git - postgresql/commitdiff
Fix security and 64-bit issues in contrib/intagg. This code could
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 27 Jan 2005 21:36:10 +0000 (21:36 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 27 Jan 2005 21:36:10 +0000 (21:36 +0000)
stand to be rewritten altogether, but for now just stick a finger in
the dike.

contrib/intagg/int_aggregate.c
contrib/intagg/int_aggregate.sql.in

index b9c39d400b59bbd702307920f722e397b72e1220..d968c2011585d1e1c0347ff0a0f9be7adeeba3ce 100644 (file)
@@ -11,8 +11,6 @@
  * This file is the property of the Digital Music Network (DMN).
  * It is being made available to users of the PostgreSQL system
  * under the BSD license.
- *
- * NOTE: This module requires sizeof(void *) to be the same as sizeof(int)
  */
 #include "postgres.h"
 
@@ -25,8 +23,6 @@
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
 #include "executor/executor.h"
-#include "utils/fcache.h"
-#include "utils/sets.h"
 #include "utils/syscache.h"
 #include "access/tupmacs.h"
 #include "access/xact.h"
@@ -38,9 +34,6 @@
 #include "utils/lsyscache.h"
 
 
-/* Uncomment this define if you are compiling for postgres 7.2.x */
-/* #define PG_7_2 */
-
 /* This is actually a postgres version of a one dimensional array */
 
 typedef struct
@@ -60,50 +53,39 @@ typedef struct callContext
 }      CTX;
 
 #define TOASTED                1
-#define START_NUM      8
-#define PGARRAY_SIZE(n) (sizeof(PGARRAY) + ((n-1)*sizeof(int4)))
+#define START_NUM      8                       /* initial size of arrays */
+#define PGARRAY_SIZE(n) (sizeof(PGARRAY) + (((n)-1)*sizeof(int4)))
 
-static PGARRAY *GetPGArray(int4 state, int fAdd);
-static PGARRAY *ShrinkPGArray(PGARRAY * p);
+static PGARRAY *GetPGArray(PGARRAY *p, int fAdd);
+static PGARRAY *ShrinkPGArray(PGARRAY *p);
 
 Datum          int_agg_state(PG_FUNCTION_ARGS);
-Datum          int_agg_final_count(PG_FUNCTION_ARGS);
 Datum          int_agg_final_array(PG_FUNCTION_ARGS);
 Datum          int_enum(PG_FUNCTION_ARGS);
 
 PG_FUNCTION_INFO_V1(int_agg_state);
-PG_FUNCTION_INFO_V1(int_agg_final_count);
 PG_FUNCTION_INFO_V1(int_agg_final_array);
 PG_FUNCTION_INFO_V1(int_enum);
 
 /*
  * Manage the aggregation state of the array
- * You need to specify the correct memory context, or it will vanish!
+ *
+ * Need to specify a suitably long-lived memory context, or it will vanish!
+ * TopTransactionContext isn't really right, but it's close enough.
  */
 static PGARRAY *
-GetPGArray(int4 state, int fAdd)
+GetPGArray(PGARRAY *p, int fAdd)
 {
-       PGARRAY    *p = (PGARRAY *) state;
-
-       if (!state)
+       if (!p)
        {
                /* New array */
                int                     cb = PGARRAY_SIZE(START_NUM);
 
                p = (PGARRAY *) MemoryContextAlloc(TopTransactionContext, cb);
-
-               if (!p)
-               {
-                       elog(ERROR, "Integer aggregator, cant allocate TopTransactionContext memory");
-                       return 0;
-               }
-
                p->a.size = cb;
                p->a.ndim = 0;
                p->a.flags = 0;
-#ifndef PG_7_2
                p->a.elemtype = INT4OID;
-#endif
                p->items = 0;
                p->lower = START_NUM;
        }
@@ -116,18 +98,6 @@ GetPGArray(int4 state, int fAdd)
                        int                     cbNew = PGARRAY_SIZE(n);
 
                        pn = (PGARRAY *) repalloc(p, cbNew);
-
-                       if (!pn)
-                       {                                       /* Realloc failed! Reallocate new block. */
-                               pn = (PGARRAY *) MemoryContextAlloc(TopTransactionContext, cbNew);
-                               if (!pn)
-                               {
-                                       elog(ERROR, "Integer aggregator, REALLY REALLY can't alloc memory");
-                                       return (PGARRAY *) NULL;
-                               }
-                               memcpy(pn, p, p->a.size);
-                               pfree(p);
-                       }
                        pn->a.size = cbNew;
                        pn->lower = n;
                        return pn;
@@ -137,7 +107,8 @@ GetPGArray(int4 state, int fAdd)
 }
 
 /* Shrinks the array to its actual size and moves it into the standard
- * memory allocation context, frees working memory     */
+ * memory allocation context, frees working memory
+ */
 static PGARRAY *
 ShrinkPGArray(PGARRAY * p)
 {
@@ -151,23 +122,16 @@ ShrinkPGArray(PGARRAY * p)
                /* use current transaction context */
                pnew = palloc(cb);
 
-               if (pnew)
-               {
-                       /*
-                        * Fix up the fields in the new structure, so Postgres
-                        * understands
-                        */
-                       memcpy(pnew, p, cb);
-                       pnew->a.size = cb;
-                       pnew->a.ndim = 1;
-                       pnew->a.flags = 0;
-#ifndef PG_7_2
-                       pnew->a.elemtype = INT4OID;
-#endif
-                       pnew->lower = 0;
-               }
-               else
-                       elog(ERROR, "Integer aggregator, can't allocate memory");
+               /*
+                * Fix up the fields in the new structure, so Postgres understands
+                */
+               memcpy(pnew, p, cb);
+               pnew->a.size = cb;
+               pnew->a.ndim = 1;
+               pnew->a.flags = 0;
+               pnew->a.elemtype = INT4OID;
+               pnew->lower = 1;
+
                pfree(p);
        }
        return pnew;
@@ -177,26 +141,37 @@ ShrinkPGArray(PGARRAY * p)
 Datum
 int_agg_state(PG_FUNCTION_ARGS)
 {
-       int4            state = PG_GETARG_INT32(0);
-       int4            value = PG_GETARG_INT32(1);
-
-       PGARRAY    *p = GetPGArray(state, 1);
+       PGARRAY    *state;
+       PGARRAY    *p;
 
-       if (!p)
-               elog(ERROR, "No aggregate storage");
-       else if (p->items >= p->lower)
-               elog(ERROR, "aggregate storage too small");
+       if (PG_ARGISNULL(0))
+               state = NULL;
        else
-               p->array[p->items++] = value;
-       PG_RETURN_INT32(p);
+               state = (PGARRAY *) PG_GETARG_POINTER(0);
+       p = GetPGArray(state, 1);
+
+       if (!PG_ARGISNULL(1))
+       {
+               int4    value = PG_GETARG_INT32(1);
+
+               if (!p)         /* internal error */
+                       elog(ERROR, "no aggregate storage");
+               else if (p->items >= p->lower)          /* internal error */
+                       elog(ERROR, "aggregate storage too small");
+               else
+                       p->array[p->items++] = value;
+       }
+       PG_RETURN_POINTER(p);
 }
 
-/* This is the final function used for the integer aggregator. It returns all the integers
- * collected as a one dimentional integer array */
+/* This is the final function used for the integer aggregator. It returns all
+ * the integers collected as a one dimensional integer array
+ */
 Datum
 int_agg_final_array(PG_FUNCTION_ARGS)
 {
-       PGARRAY    *pnew = ShrinkPGArray(GetPGArray(PG_GETARG_INT32(0), 0));
+       PGARRAY    *state = (PGARRAY *) PG_GETARG_POINTER(0);
+       PGARRAY    *pnew = ShrinkPGArray(GetPGArray(state, 0));
 
        if (pnew)
                PG_RETURN_POINTER(pnew);
@@ -213,11 +188,11 @@ int_enum(PG_FUNCTION_ARGS)
        ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo;
 
        if (!rsi || !IsA(rsi, ReturnSetInfo))
-               elog(ERROR, "No ReturnSetInfo sent! function must be declared returning a 'setof' integer");
+               elog(ERROR, "int_enum called in context that cannot accept a set");
 
        if (!p)
        {
-               elog(WARNING, "No data sent");
+               elog(WARNING, "no data sent");
                PG_RETURN_NULL();
        }
 
@@ -226,17 +201,12 @@ int_enum(PG_FUNCTION_ARGS)
                /* Allocate a working context */
                pc = (CTX *) palloc(sizeof(CTX));
 
-               /* Don't copy atribute if you don't need too */
+               /* Don't copy attribute if you don't need to */
                if (VARATT_IS_EXTENDED(p))
                {
                        /* Toasted!!! */
                        pc->p = (PGARRAY *) PG_DETOAST_DATUM_COPY(p);
                        pc->flags = TOASTED;
-                       if (!pc->p)
-                       {
-                               elog(ERROR, "Error in toaster!!! no detoasting");
-                               PG_RETURN_NULL();
-                       }
                }
                else
                {
@@ -244,11 +214,10 @@ int_enum(PG_FUNCTION_ARGS)
                        pc->p = p;
                        pc->flags = 0;
                }
-               fcinfo->context = (Node *) pc;
                pc->num = 0;
+               fcinfo->context = (Node *) pc;
        }
-       else
-/* use an existing one */
+       else    /* use an existing one */
                pc = (CTX *) fcinfo->context;
        /* Are we done yet? */
        if (pc->num >= pc->p->items)
@@ -261,8 +230,8 @@ int_enum(PG_FUNCTION_ARGS)
                rsi->isDone = ExprEndResult;
        }
        else
-/* nope, return the next value */
        {
+               /* nope, return the next value */
                int                     val = pc->p->array[pc->num++];
 
                rsi->isDone = ExprMultipleResult;
index 1d5f83e682b14f0c26f55d6cf391898fd4acfafc..caaf01afdb9818356fb7398fcb8f011e12d291d3 100644 (file)
@@ -1,36 +1,38 @@
 -- Adjust this setting to control where the objects get created.
 SET search_path = public;
 
-SET autocommit TO 'on';
-
 -- Internal function for the aggregate
 -- Is called for each item in an aggregation
-CREATE OR REPLACE FUNCTION int_agg_state (int4, int4)
-RETURNS int4
+CREATE OR REPLACE FUNCTION int_agg_state (int4[], int4)
+RETURNS int4[]
 AS 'MODULE_PATHNAME','int_agg_state'
 LANGUAGE 'C';
 
 -- Internal function for the aggregate
 -- Is called at the end of the aggregation, and returns an array.
-CREATE OR REPLACE FUNCTION int_agg_final_array (int4)
+CREATE OR REPLACE FUNCTION int_agg_final_array (int4[])
 RETURNS int4[]
 AS 'MODULE_PATHNAME','int_agg_final_array'
-LANGUAGE 'C';
+LANGUAGE 'C' STRICT;
 
--- The aggration funcion.
+-- The aggregate function itself
 -- uses the above functions to create an array of integers from an aggregation.
-CREATE OR REPLACE AGGREGATE int_array_aggregate (
+CREATE AGGREGATE int_array_aggregate (
        BASETYPE = int4,
        SFUNC = int_agg_state,
-       STYPE = int4,
-       FINALFUNC = int_agg_final_array,
-       INITCOND = 0
+       STYPE = int4[],
+       FINALFUNC = int_agg_final_array
 );
 
+-- The aggregate component functions are not designed to be called
+-- independently, so disable public access to them
+REVOKE ALL ON FUNCTION int_agg_state (int4[], int4) FROM PUBLIC;
+REVOKE ALL ON FUNCTION int_agg_final_array (int4[]) FROM PUBLIC;
+
 -- The enumeration function
--- returns each element in a one dimentional integer array
+-- returns each element in a one dimensional integer array
 -- as a row.
 CREATE OR REPLACE FUNCTION int_array_enum(int4[])
 RETURNS setof integer
 AS 'MODULE_PATHNAME','int_enum'
-LANGUAGE 'C';
+LANGUAGE 'C' IMMUTABLE STRICT;