From: Tom Lane Date: Thu, 27 Jan 2005 21:36:10 +0000 (+0000) Subject: Fix security and 64-bit issues in contrib/intagg. This code could X-Git-Tag: REL7_3_9~5 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=05dadca2a0921be9586d0ef40ac97cf67f28c187;p=postgresql Fix security and 64-bit issues in contrib/intagg. This code could stand to be rewritten altogether, but for now just stick a finger in the dike. --- diff --git a/contrib/intagg/int_aggregate.c b/contrib/intagg/int_aggregate.c index b9c39d400b..d968c20115 100644 --- a/contrib/intagg/int_aggregate.c +++ b/contrib/intagg/int_aggregate.c @@ -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; diff --git a/contrib/intagg/int_aggregate.sql.in b/contrib/intagg/int_aggregate.sql.in index 1d5f83e682..caaf01afdb 100644 --- a/contrib/intagg/int_aggregate.sql.in +++ b/contrib/intagg/int_aggregate.sql.in @@ -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;