]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Guard against overflow in "left" fields of query_int and ltxtquery.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 11 May 2026 12:13:50 +0000 (05:13 -0700)
committerNoah Misch <noah@leadboat.com>
Mon, 11 May 2026 12:13:50 +0000 (05:13 -0700)
contrib/intarray's query_int type uses an int16 field to hold the
offset from a binary operator node to its left operand.  However, it
allows the number of nodes to be as much as will fit in MaxAllocSize,
so there is a risk of overflowing int16 depending on the precise shape
of the tree.  Simple right-associative cases like "a | b | c | ..."
work fine, so we should not solve this by restricting the overall
number of nodes.  Instead add a direct test of whether each individual
offset is too large.

contrib/ltree's ltxtquery type uses essentially the same logic and
has the same 16-bit restriction.

(The core backend's tsquery.c has a variant of this logic too, but
in that case the target field is 32 bits, so it is okay so long
as varlena datums are restricted to 1GB.)

In v16 and up, these types support soft error reporting, so we have
to complicate the recursive findoprnd function's API a bit to allow
the complaint to be reported softly.  v14/v15 don't need that.

Undocumented and overcomplicated code like this makes my head hurt,
so add some comments and simplify while at it.

Reported-by: Xint Code
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Backpatch-through: 14
Security: CVE-2026-6473

contrib/intarray/_int_bool.c
contrib/intarray/expected/_int.out
contrib/intarray/sql/_int.sql
contrib/ltree/expected/ltree.out
contrib/ltree/ltxtquery_io.c
contrib/ltree/sql/ltree.sql

index 3ed88af76d777e3be65a6a188e33e24006ff266a..d415a3dd6a491777573079148a7992bbe17fdd07 100644 (file)
@@ -435,35 +435,66 @@ boolop(PG_FUNCTION_ARGS)
        PG_RETURN_BOOL(result);
 }
 
+/*
+ * Recursively fill the "left" fields of an ITEM array that represents
+ * a valid postfix tree.
+ *
+ *     ptr: starting element of array
+ *     pos: in/out argument, the array index this call is responsible to fill
+ *
+ * At exit, *pos has been decremented to point before the sub-tree whose
+ * top is the entry-time value of *pos.
+ */
 static void
 findoprnd(ITEM *ptr, int32 *pos)
 {
+       int32           mypos;
+
        /* since this function recurses, it could be driven to stack overflow. */
        check_stack_depth();
 
+       /* get the position this call is supposed to update */
+       mypos = *pos;
+       Assert(mypos >= 0);
+
+       /* in all cases, we should decrement *pos to advance over this item */
+       (*pos)--;
+
 #ifdef BS_DEBUG
-       elog(DEBUG3, (ptr[*pos].type == OPR) ?
-                "%d  %c" : "%d  %d", *pos, ptr[*pos].val);
+       elog(DEBUG3, (ptr[mypos].type == OPR) ?
+                "%d  %c" : "%d  %d", mypos, ptr[mypos].val);
 #endif
-       if (ptr[*pos].type == VAL)
+
+       if (ptr[mypos].type == VAL)
        {
-               ptr[*pos].left = 0;
-               (*pos)--;
+               /* base case: a VAL has no operand, so just set its left to zero */
+               ptr[mypos].left = 0;
        }
-       else if (ptr[*pos].val == (int32) '!')
+       else if (ptr[mypos].val == (int32) '!')
        {
-               ptr[*pos].left = -1;
-               (*pos)--;
+               /* unary operator, likewise easy: operand is just before it */
+               ptr[mypos].left = -1;
+               /* recurse to scan operand */
                findoprnd(ptr, pos);
        }
        else
        {
-               ITEM       *curitem = &ptr[*pos];
-               int32           tmp = *pos;
+               /* binary operator */
+               int32           delta;
 
-               (*pos)--;
+               /* recurse to scan right operand */
                findoprnd(ptr, pos);
-               curitem->left = *pos - tmp;
+               /* we must fill left with offset to left operand's top */
+               /* abs(delta) < QUERYTYPEMAXITEMS, so it can't overflow ... */
+               delta = *pos - mypos;
+               /* ... but it might be too large to fit in the 16-bit left field */
+               Assert(delta < 0);
+               if (unlikely(delta < PG_INT16_MIN))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                        errmsg("query_int expression is too complex")));
+               ptr[mypos].left = (int16) delta;
+               /* recurse to scan left operand */
                findoprnd(ptr, pos);
        }
 }
@@ -513,6 +544,7 @@ bqarr_in(PG_FUNCTION_ARGS)
        query->size = state.num;
        ptr = GETQUERY(query);
 
+       /* fill the query array from the data makepol constructed */
        for (i = state.num - 1; i >= 0; i--)
        {
                ptr[i].type = state.str->type;
@@ -522,8 +554,12 @@ bqarr_in(PG_FUNCTION_ARGS)
                state.str = tmp;
        }
 
+       /* now fill the "left" fields */
        pos = query->size - 1;
        findoprnd(ptr, &pos);
+       /* if successful, findoprnd should have scanned the whole array */
+       Assert(pos == -1);
+
 #ifdef BS_DEBUG
        initStringInfo(&pbuf);
        for (i = 0; i < query->size; i++)
index 64d8878763283e0d75ba81425d3f779b962b1128..964721c70aa263f53ec9e98b1891be0efb3101f6 100644 (file)
@@ -398,6 +398,9 @@ SELECT '1&(2&(4&(5|!6)))'::query_int;
  1 & 2 & 4 & ( 5 | !6 )
 (1 row)
 
+SELECT (SELECT '0 | ' || string_agg(i::text, ' & ')
+        FROM generate_series(1, 17000) AS i)::query_int;
+ERROR:  query_int expression is too complex
 CREATE TABLE test__int( a int[] );
 \copy test__int from 'data/test__int.data'
 ANALYZE test__int;
index ba4c298151a1ca0f68ac503c633d4597cc3fd22c..efc4f1685b72944f025f79aa5d90e796eabe16bb 100644 (file)
@@ -74,6 +74,8 @@ SELECT '1&(2&(4&(5&6)))'::query_int;
 SELECT '1&2&4&5&6'::query_int;
 SELECT '1&(2&(4&(5|6)))'::query_int;
 SELECT '1&(2&(4&(5|!6)))'::query_int;
+SELECT (SELECT '0 | ' || string_agg(i::text, ' & ')
+        FROM generate_series(1, 17000) AS i)::query_int;
 
 
 CREATE TABLE test__int( a int[] );
index 02cce5d925a9713acbdfad28e8b8edb21bdcf2d9..893838e1b7b823d3ac0828cc573d6653ec2ed60c 100644 (file)
@@ -1267,6 +1267,9 @@ SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery;
  f
 (1 row)
 
+SELECT (SELECT 'a | ' || string_agg('b', ' & ')
+        FROM generate_series(1, 17000) AS i)::ltxtquery;
+ERROR:  ltxtquery is too large
 --arrays
 SELECT '{1.2.3}'::ltree[] @> '1.2.3.4';
  ?column? 
index bda2d971021798bd3c115b5a28a851c95fe9f9e3..a4e2e2b33adf2ec90e2e41aa65954a76df7a2d5d 100644 (file)
@@ -270,31 +270,60 @@ makepol(QPRS_STATE *state)
        return END;
 }
 
+/*
+ * Recursively fill the "left" fields of an ITEM array that represents
+ * a valid postfix tree.
+ *
+ *     ptr: starting element of array
+ *     pos: in/out argument, the array index this call is responsible to fill
+ *
+ * At exit, *pos has been incremented to point after the sub-tree whose
+ * top is the entry-time value of *pos.
+ */
 static void
 findoprnd(ITEM *ptr, int32 *pos)
 {
+       int32           mypos;
+
        /* since this function recurses, it could be driven to stack overflow. */
        check_stack_depth();
 
-       if (ptr[*pos].type == VAL || ptr[*pos].type == VALTRUE)
+       /* get the position this call is supposed to update */
+       mypos = *pos;
+
+       /* in all cases, we should increment *pos to advance over this item */
+       (*pos)++;
+
+       if (ptr[mypos].type == VAL || ptr[mypos].type == VALTRUE)
        {
-               ptr[*pos].left = 0;
-               (*pos)++;
+               /* base case: a VAL has no operand, so just set its left to zero */
+               ptr[mypos].left = 0;
        }
-       else if (ptr[*pos].val == (int32) '!')
+       else if (ptr[mypos].val == (int32) '!')
        {
-               ptr[*pos].left = 1;
-               (*pos)++;
+               /* unary operator, likewise easy: operand is just after it */
+               ptr[mypos].left = 1;
+               /* recurse to scan operand */
                findoprnd(ptr, pos);
        }
        else
        {
-               ITEM       *curitem = &ptr[*pos];
-               int32           tmp = *pos;
+               /* binary operator */
+               int32           delta;
 
-               (*pos)++;
+               /* recurse to scan right operand */
                findoprnd(ptr, pos);
-               curitem->left = *pos - tmp;
+               /* we must fill left with offset to left operand's top */
+               /* delta can't overflow, see LTXTQUERY_TOO_BIG ... */
+               delta = *pos - mypos;
+               /* ... but it might be too large to fit in the 16-bit left field */
+               Assert(delta > 0);
+               if (unlikely(delta > PG_INT16_MAX))
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                        errmsg("ltxtquery is too large")));
+               ptr[mypos].left = (int16) delta;
+               /* recurse to scan left operand */
                findoprnd(ptr, pos);
        }
 }
@@ -371,6 +400,8 @@ queryin(char *buf)
        /* set left operand's position for every operator */
        pos = 0;
        findoprnd(ptr, &pos);
+       /* if successful, findoprnd should have scanned the whole array */
+       Assert(pos == state.num);
 
        return query;
 }
index 647f7f475f9b5f213e078969838c8f01c90c3d11..ee20092d5eb64ed3ab0ed6c0c6a9f4cb308d63fe 100644 (file)
@@ -246,6 +246,9 @@ SELECT 'tree.awdfg'::ltree @ 'tree & aWdfg@'::ltxtquery;
 SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_qw%*'::ltxtquery;
 SELECT 'tree.awdfg_qwerty'::ltree @ 'tree & aw_rw%*'::ltxtquery;
 
+SELECT (SELECT 'a | ' || string_agg('b', ' & ')
+        FROM generate_series(1, 17000) AS i)::ltxtquery;
+
 --arrays
 
 SELECT '{1.2.3}'::ltree[] @> '1.2.3.4';