From 98be8a6eaa9fe917f6be9e121187b912e19ab484 Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Mon, 17 Feb 2014 09:33:31 -0500 Subject: [PATCH] Predict integer overflow to avoid buffer overruns. Several functions, mostly type input functions, calculated an allocation size such that the calculation wrapped to a small positive value when arguments implied a sufficiently-large requirement. Writes past the end of the inadvertent small allocation followed shortly thereafter. Coverity identified the path_in() vulnerability; code inspection led to the rest. In passing, add check_stack_depth() to prevent stack overflow in related functions. Back-patch to 8.4 (all supported versions). The non-comment hstore changes touch code that did not exist in 8.4, so that part stops at 9.0. Noah Misch and Heikki Linnakangas, reviewed by Tom Lane. Security: CVE-2014-0064 --- contrib/intarray/_int.h | 2 ++ contrib/intarray/_int_bool.c | 9 ++++++++ contrib/ltree/ltree.h | 3 +++ contrib/ltree/ltree_io.c | 11 ++++++++++ contrib/ltree/ltxtquery_io.c | 13 ++++++++++- src/backend/utils/adt/geo_ops.c | 30 ++++++++++++++++++++++++-- src/backend/utils/adt/tsquery.c | 7 +++++- src/backend/utils/adt/tsquery_util.c | 5 +++++ src/backend/utils/adt/txid.c | 15 +++++-------- src/backend/utils/adt/varbit.c | 32 ++++++++++++++++++++++++++-- src/include/tsearch/ts_type.h | 3 +++ src/include/utils/varbit.h | 7 ++++++ 12 files changed, 121 insertions(+), 16 deletions(-) diff --git a/contrib/intarray/_int.h b/contrib/intarray/_int.h index 35dbb54796f..f448ef11aad 100644 --- a/contrib/intarray/_int.h +++ b/contrib/intarray/_int.h @@ -5,6 +5,7 @@ #define ___INT_H__ #include "utils/array.h" +#include "utils/memutils.h" /* number ranges for compression */ #define MAXNUMRANGE 100 @@ -142,6 +143,7 @@ typedef struct #define HDRSIZEQT (VARHDRSZ + sizeof(int4)) #define COMPUTESIZE(size) ( HDRSIZEQT + size * sizeof(ITEM) ) +#define QUERYTYPEMAXITEMS ((MaxAllocSize - HDRSIZEQT) / sizeof(ITEM)) #define GETQUERY(x) (ITEM*)( (char*)(x)+HDRSIZEQT ) #define END 0 diff --git a/contrib/intarray/_int_bool.c b/contrib/intarray/_int_bool.c index 7a478faa83b..468a53a7bbb 100644 --- a/contrib/intarray/_int_bool.c +++ b/contrib/intarray/_int_bool.c @@ -416,6 +416,9 @@ boolop(PG_FUNCTION_ARGS) static void findoprnd(ITEM *ptr, int4 *pos) { + /* since this function recurses, it could be driven to stack overflow. */ + check_stack_depth(); + #ifdef BS_DEBUG elog(DEBUG3, (ptr[*pos].type == OPR) ? "%d %c" : "%d %d", *pos, ptr[*pos].val); @@ -476,7 +479,13 @@ bqarr_in(PG_FUNCTION_ARGS) (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("empty query"))); + if (state.num > QUERYTYPEMAXITEMS) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("number of query items (%d) exceeds the maximum allowed (%d)", + state.num, (int) QUERYTYPEMAXITEMS))); commonlen = COMPUTESIZE(state.num); + query = (QUERYTYPE *) palloc(commonlen); SET_VARSIZE(query, commonlen); query->size = state.num; diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h index f16c6f9a32a..c3644e63ea0 100644 --- a/contrib/ltree/ltree.h +++ b/contrib/ltree/ltree.h @@ -6,6 +6,7 @@ #include "postgres.h" #include "fmgr.h" #include "tsearch/ts_locale.h" +#include "utils/memutils.h" typedef struct { @@ -112,6 +113,8 @@ typedef struct #define HDRSIZEQT MAXALIGN(VARHDRSZ + sizeof(int4)) #define COMPUTESIZE(size,lenofoperand) ( HDRSIZEQT + (size) * sizeof(ITEM) + (lenofoperand) ) +#define LTXTQUERY_TOO_BIG(size,lenofoperand) \ + ((size) > (MaxAllocSize - HDRSIZEQT - (lenofoperand)) / sizeof(ITEM)) #define GETQUERY(x) (ITEM*)( (char*)(x)+HDRSIZEQT ) #define GETOPERAND(x) ( (char*)GETQUERY(x) + ((ltxtquery*)x)->size * sizeof(ITEM) ) diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.c index a88eb16cb97..f92e9aaa7ea 100644 --- a/contrib/ltree/ltree_io.c +++ b/contrib/ltree/ltree_io.c @@ -8,6 +8,7 @@ #include #include "ltree.h" +#include "utils/memutils.h" #include "crc32.h" PG_FUNCTION_INFO_V1(ltree_in); @@ -64,6 +65,11 @@ ltree_in(PG_FUNCTION_ARGS) ptr += charlen; } + if (num + 1 > MaxAllocSize / sizeof(nodeitem)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("number of levels (%d) exceeds the maximum allowed (%d)", + num + 1, (int) (MaxAllocSize / sizeof(nodeitem))))); list = lptr = (nodeitem *) palloc(sizeof(nodeitem) * (num + 1)); ptr = buf; while (*ptr) @@ -228,6 +234,11 @@ lquery_in(PG_FUNCTION_ARGS) } num++; + if (num > MaxAllocSize / ITEMSIZE) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("number of levels (%d) exceeds the maximum allowed (%d)", + num, (int) (MaxAllocSize / ITEMSIZE)))); curqlevel = tmpql = (lquery_level *) palloc0(ITEMSIZE * num); ptr = buf; while (*ptr) diff --git a/contrib/ltree/ltxtquery_io.c b/contrib/ltree/ltxtquery_io.c index d9163babf74..2cbcc89f507 100644 --- a/contrib/ltree/ltxtquery_io.c +++ b/contrib/ltree/ltxtquery_io.c @@ -9,6 +9,7 @@ #include "crc32.h" #include "ltree.h" +#include "miscadmin.h" PG_FUNCTION_INFO_V1(ltxtq_in); Datum ltxtq_in(PG_FUNCTION_ARGS); @@ -213,6 +214,9 @@ makepol(QPRS_STATE *state) int4 lenstack = 0; uint16 flag = 0; + /* since this function recurses, it could be driven to stack overflow */ + check_stack_depth(); + while ((type = gettoken_query(state, &val, &lenval, &strval, &flag)) != END) { switch (type) @@ -277,6 +281,9 @@ makepol(QPRS_STATE *state) static void findoprnd(ITEM *ptr, int4 *pos) { + /* since this function recurses, it could be driven to stack overflow. */ + check_stack_depth(); + if (ptr[*pos].type == VAL || ptr[*pos].type == VALTRUE) { ptr[*pos].left = 0; @@ -341,8 +348,12 @@ queryin(char *buf) errmsg("syntax error"), errdetail("Empty query."))); - /* make finish struct */ + if (LTXTQUERY_TOO_BIG(state.num, state.sumlen)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("ltxtquery is too large"))); commonlen = COMPUTESIZE(state.num, state.sumlen); + query = (ltxtquery *) palloc(commonlen); SET_VARSIZE(query, commonlen); query->size = state.num; diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c index c1276b78c12..3ffab5eb2de 100644 --- a/src/backend/utils/adt/geo_ops.c +++ b/src/backend/utils/adt/geo_ops.c @@ -1401,6 +1401,7 @@ path_in(PG_FUNCTION_ARGS) char *s; int npts; int size; + int base_size; int depth = 0; if ((npts = pair_count(str, ',')) <= 0) @@ -1419,7 +1420,15 @@ path_in(PG_FUNCTION_ARGS) depth++; } - size = offsetof(PATH, p[0]) +sizeof(path->p[0]) * npts; + base_size = sizeof(path->p[0]) * npts; + size = offsetof(PATH, p[0]) + base_size; + + /* Check for integer overflow */ + if (base_size / npts != sizeof(path->p[0]) || size <= base_size) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("too many points requested"))); + path = (PATH *) palloc(size); SET_VARSIZE(path, size); @@ -3439,6 +3448,7 @@ poly_in(PG_FUNCTION_ARGS) POLYGON *poly; int npts; int size; + int base_size; int isopen; char *s; @@ -3447,7 +3457,15 @@ poly_in(PG_FUNCTION_ARGS) (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION), errmsg("invalid input syntax for type polygon: \"%s\"", str))); - size = offsetof(POLYGON, p[0]) +sizeof(poly->p[0]) * npts; + base_size = sizeof(poly->p[0]) * npts; + size = offsetof(POLYGON, p[0]) + base_size; + + /* Check for integer overflow */ + if (base_size / npts != sizeof(poly->p[0]) || size <= base_size) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("too many points requested"))); + poly = (POLYGON *) palloc0(size); /* zero any holes */ SET_VARSIZE(poly, size); @@ -4216,6 +4234,10 @@ path_poly(PG_FUNCTION_ARGS) (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("open path cannot be converted to polygon"))); + /* + * Never overflows: the old size fit in MaxAllocSize, and the new size is + * just a small constant larger. + */ size = offsetof(POLYGON, p[0]) +sizeof(poly->p[0]) * path->npts; poly = (POLYGON *) palloc(size); @@ -4321,6 +4343,10 @@ poly_path(PG_FUNCTION_ARGS) int size; int i; + /* + * Never overflows: the old size fit in MaxAllocSize, and the new size is + * smaller by a small constant. + */ size = offsetof(PATH, p[0]) +sizeof(path->p[0]) * poly->npts; path = (PATH *) palloc(size); diff --git a/src/backend/utils/adt/tsquery.c b/src/backend/utils/adt/tsquery.c index b8d8128de39..e821f7924f8 100644 --- a/src/backend/utils/adt/tsquery.c +++ b/src/backend/utils/adt/tsquery.c @@ -517,8 +517,13 @@ parse_tsquery(char *buf, return query; } - /* Pack the QueryItems in the final TSQuery struct to return to caller */ + if (TSQUERY_TOO_BIG(list_length(state.polstr), state.sumlen)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("tsquery is too large"))); commonlen = COMPUTESIZE(list_length(state.polstr), state.sumlen); + + /* Pack the QueryItems in the final TSQuery struct to return to caller */ query = (TSQuery) palloc0(commonlen); SET_VARSIZE(query, commonlen); query->size = list_length(state.polstr); diff --git a/src/backend/utils/adt/tsquery_util.c b/src/backend/utils/adt/tsquery_util.c index d4a2ede1f62..dfe5779daf5 100644 --- a/src/backend/utils/adt/tsquery_util.c +++ b/src/backend/utils/adt/tsquery_util.c @@ -334,6 +334,11 @@ QTN2QT(QTNode *in) QTN2QTState state; cntsize(in, &sumlen, &nnode); + + if (TSQUERY_TOO_BIG(nnode, sumlen)) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("tsquery is too large"))); len = COMPUTESIZE(nnode, sumlen); out = (TSQuery) palloc0(len); diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c index a4a5b866768..fa01b9a5fd4 100644 --- a/src/backend/utils/adt/txid.c +++ b/src/backend/utils/adt/txid.c @@ -26,6 +26,7 @@ #include "funcapi.h" #include "libpq/pqformat.h" #include "utils/builtins.h" +#include "utils/memutils.h" #include "utils/snapmgr.h" @@ -70,6 +71,8 @@ typedef struct #define TXID_SNAPSHOT_SIZE(nxip) \ (offsetof(TxidSnapshot, xip) + sizeof(txid) * (nxip)) +#define TXID_SNAPSHOT_MAX_NXIP \ + ((MaxAllocSize - offsetof(TxidSnapshot, xip)) / sizeof(txid)) /* * Epoch values from xact.c @@ -445,20 +448,12 @@ txid_snapshot_recv(PG_FUNCTION_ARGS) txid last = 0; int nxip; int i; - int avail; - int expect; txid xmin, xmax; - /* - * load nxip and check for nonsense. - * - * (nxip > avail) check is against int overflows in 'expect'. - */ + /* load and validate nxip */ nxip = pq_getmsgint(buf, 4); - avail = buf->len - buf->cursor; - expect = 8 + 8 + nxip * 8; - if (nxip < 0 || nxip > avail || expect > avail) + if (nxip < 0 || nxip > TXID_SNAPSHOT_MAX_NXIP) goto bad_format; xmin = pq_getmsgint64(buf); diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c index aebc2baf1f5..3c15658c90f 100644 --- a/src/backend/utils/adt/varbit.c +++ b/src/backend/utils/adt/varbit.c @@ -138,12 +138,22 @@ bit_in(PG_FUNCTION_ARGS) sp = input_string; } + /* + * Determine bitlength from input string. MaxAllocSize ensures a regular + * input is small enough, but we must check hex input. + */ slen = strlen(sp); - /* Determine bitlength from input string */ if (bit_not_hex) bitlen = slen; else + { + if (slen > VARBITMAXLEN / 4) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("bit string length exceeds the maximum allowed (%d)", + VARBITMAXLEN))); bitlen = slen * 4; + } /* * Sometimes atttypmod is not supplied. If it is supplied we need to make @@ -436,12 +446,22 @@ varbit_in(PG_FUNCTION_ARGS) sp = input_string; } + /* + * Determine bitlength from input string. MaxAllocSize ensures a regular + * input is small enough, but we must check hex input. + */ slen = strlen(sp); - /* Determine bitlength from input string */ if (bit_not_hex) bitlen = slen; else + { + if (slen > VARBITMAXLEN / 4) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("bit string length exceeds the maximum allowed (%d)", + VARBITMAXLEN))); bitlen = slen * 4; + } /* * Sometimes atttypmod is not supplied. If it is supplied we need to make @@ -520,6 +540,9 @@ varbit_in(PG_FUNCTION_ARGS) /* varbit_out - * Prints the string as bits to preserve length accurately + * + * XXX varbit_recv() and hex input to varbit_in() can load a value that this + * cannot emit. Consider using hex output for such values. */ Datum varbit_out(PG_FUNCTION_ARGS) @@ -886,6 +909,11 @@ bitcat(PG_FUNCTION_ARGS) bitlen1 = VARBITLEN(arg1); bitlen2 = VARBITLEN(arg2); + if (bitlen1 > VARBITMAXLEN - bitlen2) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("bit string length exceeds the maximum allowed (%d)", + VARBITMAXLEN))); bytelen = VARBITTOTALLEN(bitlen1 + bitlen2); result = (VarBit *) palloc(bytelen); diff --git a/src/include/tsearch/ts_type.h b/src/include/tsearch/ts_type.h index 60c16553eaf..b9ca6cece60 100644 --- a/src/include/tsearch/ts_type.h +++ b/src/include/tsearch/ts_type.h @@ -13,6 +13,7 @@ #define _PG_TSTYPE_H_ #include "fmgr.h" +#include "utils/memutils.h" #include "utils/pg_crc.h" @@ -242,6 +243,8 @@ typedef TSQueryData *TSQuery; * QueryItems, and lenofoperand is the total length of all operands */ #define COMPUTESIZE(size, lenofoperand) ( HDRSIZETQ + (size) * sizeof(QueryItem) + (lenofoperand) ) +#define TSQUERY_TOO_BIG(size, lenofoperand) \ + ((size) > (MaxAllocSize - HDRSIZETQ - (lenofoperand)) / sizeof(QueryItem)) /* Returns a pointer to the first QueryItem in a TSQuery */ #define GETQUERY(x) ((QueryItem*)( (char*)(x)+HDRSIZETQ )) diff --git a/src/include/utils/varbit.h b/src/include/utils/varbit.h index d316a0f093b..c8dd5ede9cd 100644 --- a/src/include/utils/varbit.h +++ b/src/include/utils/varbit.h @@ -15,6 +15,8 @@ #ifndef VARBIT_H #define VARBIT_H +#include + #include "fmgr.h" /* @@ -53,6 +55,11 @@ typedef struct /* Number of bytes needed to store a bit string of a given length */ #define VARBITTOTALLEN(BITLEN) (((BITLEN) + BITS_PER_BYTE-1)/BITS_PER_BYTE + \ VARHDRSZ + VARBITHDRSZ) +/* + * Maximum number of bits. Several code sites assume no overflow from + * computing bitlen + X; VARBITTOTALLEN() has the largest such X. + */ +#define VARBITMAXLEN (INT_MAX - BITS_PER_BYTE + 1) /* pointer beyond the end of the bit string (like end() in STL containers) */ #define VARBITEND(PTR) (((bits8 *) (PTR)) + VARSIZE(PTR)) /* Mask that will cover exactly one byte, i.e. BITS_PER_BYTE bits */