From 8fe4e659ab62a3b4fa82cc88e18b61db08b9bd35 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 5 Sep 2026 21:42:26 +0000 Subject: [PATCH v2 2/2] test_bitmapset: Check argument type before calling stringToNode() The SQL wrappers of this module decoded their text arguments with the generic stringToNode(), casting whatever came out to a Bitmapset. stringToNode() assumes its input to be valid: the read routines of readfuncs.c fetch tokens with pg_strtok() and feed them to atoi() without checking for NULL, so any other node type name, like '{QUERY}', crashes the backend on the first missing field. Strings for other node types that do read fine, like '42' or '(i 1 2)', were then misinterpreted as a Bitmapset. The functions are executable by any user, so this was reachable without any privileges. To fix, look at the leading tokens of the string before handing it to stringToNode(), and reject anything but the two forms nodeToString() writes for a Bitmapset: "<>" for an empty set, and "(b member ...)" otherwise. The code reading the latter in nodeRead() checks every token it consumes, so no other read routine is ever reached. Oversight in commit 00c3d87a5ca. --- .../expected/test_bitmapset.out | 27 +++++++++++ .../test_bitmapset/sql/test_bitmapset.sql | 12 +++++ .../modules/test_bitmapset/test_bitmapset.c | 48 +++++++++++++++++-- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/src/test/modules/test_bitmapset/expected/test_bitmapset.out b/src/test/modules/test_bitmapset/expected/test_bitmapset.out index f8f5cf5e7f0..7406ed929f9 100644 --- a/src/test/modules/test_bitmapset/expected/test_bitmapset.out +++ b/src/test/modules/test_bitmapset/expected/test_bitmapset.out @@ -1656,4 +1656,31 @@ SELECT test_random_offset_operations(NULL, 1000, 0, 1024) AS result; 1000 (1 row) +-- malformed inputs, rejected before reaching the read routines of other +-- node types +SELECT test_bms_num_members('{QUERY}'); -- error +ERROR: argument is not a serialized Bitmapset +SELECT test_bms_num_members('42'); -- error +ERROR: argument is not a serialized Bitmapset +SELECT test_bms_num_members('(i 1 2)'); -- error +ERROR: argument is not a serialized Bitmapset +SELECT test_bms_num_members('(b 1'); -- error +ERROR: unterminated Bitmapset structure +SELECT test_bms_num_members('(b x)'); -- error +ERROR: unrecognized integer: "x" +SELECT test_bms_num_members(''); -- error +ERROR: argument is not a serialized Bitmapset +-- empty set, as written by nodeToString() +SELECT test_bms_num_members('<>') AS result; + result +-------- + 0 +(1 row) + +SELECT test_bms_copy('<>') AS result; + result +-------- + <> +(1 row) + DROP EXTENSION test_bitmapset; diff --git a/src/test/modules/test_bitmapset/sql/test_bitmapset.sql b/src/test/modules/test_bitmapset/sql/test_bitmapset.sql index d44cda114a4..7096fa60fbe 100644 --- a/src/test/modules/test_bitmapset/sql/test_bitmapset.sql +++ b/src/test/modules/test_bitmapset/sql/test_bitmapset.sql @@ -426,4 +426,16 @@ SELECT test_random_operations(NULL, 10000, 0, 81920) > 0 AS result; -- perform some random tests on bms_offset_members() SELECT test_random_offset_operations(NULL, 1000, 0, 1024) AS result; +-- malformed inputs, rejected before reaching the read routines of other +-- node types +SELECT test_bms_num_members('{QUERY}'); -- error +SELECT test_bms_num_members('42'); -- error +SELECT test_bms_num_members('(i 1 2)'); -- error +SELECT test_bms_num_members('(b 1'); -- error +SELECT test_bms_num_members('(b x)'); -- error +SELECT test_bms_num_members(''); -- error +-- empty set, as written by nodeToString() +SELECT test_bms_num_members('<>') AS result; +SELECT test_bms_copy('<>') AS result; + DROP EXTENSION test_bitmapset; diff --git a/src/test/modules/test_bitmapset/test_bitmapset.c b/src/test/modules/test_bitmapset/test_bitmapset.c index af8c664949e..310dcf1a122 100644 --- a/src/test/modules/test_bitmapset/test_bitmapset.c +++ b/src/test/modules/test_bitmapset/test_bitmapset.c @@ -24,6 +24,7 @@ #include "nodes/bitmapset.h" #include "nodes/nodes.h" #include "nodes/pg_list.h" +#include "nodes/readfuncs.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/timestamp.h" @@ -86,16 +87,57 @@ PG_FUNCTION_INFO_V1(test_random_offset_operations); #expr, __FILE__, __LINE__); \ } while (0) -/* Encode/Decode to/from TEXT and Bitmapset */ +/* Encode a Bitmapset into its serialized representation */ #define BITMAPSET_TO_TEXT(bms) cstring_to_text(nodeToString(bms)) -#define TEXT_TO_BITMAPSET(str) ((Bitmapset *) stringToNode(text_to_cstring(str))) + +/* + * Decode a Bitmapset from its serialized representation via stringToNode(). + * + * stringToNode() assumes its input to be valid, and the read routines of + * readfuncs.c it dispatches to for other node types do not check for missing + * tokens, so handing it a string naming any of those could crash the backend. + * Hence, look at the leading tokens first and accept only the two forms + * written by nodeToString() for a Bitmapset: "<>" for an empty set, which + * pg_strtok() returns as a token of length zero, or "(b member ...)". The + * code reading the latter in nodeRead() checks every token it consumes. + */ +static Bitmapset * +text_to_bitmapset(text *txt) +{ + char *str = text_to_cstring(txt); + ReadNodeContext ctx = {.str = str}; + const char *token; + int length; + bool is_bitmapset = false; + Node *node; + + token = pg_strtok(&ctx, &length); + if (token != NULL && length == 0) + return NULL; + + if (token != NULL && length == 1 && token[0] == '(') + { + token = pg_strtok(&ctx, &length); + is_bitmapset = token != NULL && length == 1 && token[0] == 'b'; + } + + if (!is_bitmapset) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("argument is not a serialized Bitmapset"))); + + node = stringToNode(str); + Assert(node == NULL || IsA(node, Bitmapset)); + + return (Bitmapset *) node; +} /* * Helper macro to fetch text parameters as Bitmapsets. SQL-NULL means empty * set. */ #define PG_ARG_GETBITMAPSET(n) \ - (PG_ARGISNULL(n) ? NULL : TEXT_TO_BITMAPSET(PG_GETARG_TEXT_PP(n))) + (PG_ARGISNULL(n) ? NULL : text_to_bitmapset(PG_GETARG_TEXT_PP(n))) /* * Helper macro to handle converting sets back to text, returning the -- 2.43.0