From 0bf4a47d9d8754108de965ac64a27efb4530a92d Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 5 Sep 2026 21:34:56 +0000 Subject: [PATCH 1/2] test_extensible: Reject other node types before calling stringToNode() The SQL wrappers of this module decode their text argument with stringToNode(), and only then check that the result is a TestExtNode. stringToNode() assumes its input to be valid, though: 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}' or '{VAR}', crashes the backend on the first missing field. 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 an EXTENSIBLENODE named TestExtNode, so that only the nodeRead callback of this module, which does check for missing tokens, ever sees the rest of the string. Oversight in commit 0e944fe3579. --- .../expected/test_extensible.out | 11 ++++++ .../test_extensible/sql/test_extensible.sql | 7 ++++ .../modules/test_extensible/test_extensible.c | 39 ++++++++++++++++--- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/test/modules/test_extensible/expected/test_extensible.out b/src/test/modules/test_extensible/expected/test_extensible.out index 1fa3b6d2c55..091bdafaa79 100644 --- a/src/test/modules/test_extensible/expected/test_extensible.out +++ b/src/test/modules/test_extensible/expected/test_extensible.out @@ -91,6 +91,17 @@ SELECT test_ext_node_equal(test_ext_node_make('1234'::oid, 2), -- correct node type, missing field SELECT test_ext_node_get_relid('{EXTENSIBLENODE :extnodename TestExtNode}'); ERROR: unexpected end of "TestExtNode" +-- other node types, rejected before reaching their core read routines +SELECT test_ext_node_get_relid('{QUERY}'); +ERROR: argument is not a serialized "TestExtNode" +SELECT test_ext_node_copy('{VAR}'); +ERROR: argument is not a serialized "TestExtNode" +-- extensible node of another kind +SELECT test_ext_node_get_relid('{EXTENSIBLENODE :extnodename NoSuchExtNode}'); +ERROR: argument is not a serialized "TestExtNode" +-- not a node string at all +SELECT test_ext_node_get_relid('42'); +ERROR: argument is not a serialized "TestExtNode" -- CustomScan tests CREATE TABLE test_extensible_tbl (id integer, val text); INSERT INTO test_extensible_tbl VALUES (1, 'one'), (2, 'two'), (3, 'three'); diff --git a/src/test/modules/test_extensible/sql/test_extensible.sql b/src/test/modules/test_extensible/sql/test_extensible.sql index b23bd1cd087..347c747a6bd 100644 --- a/src/test/modules/test_extensible/sql/test_extensible.sql +++ b/src/test/modules/test_extensible/sql/test_extensible.sql @@ -33,6 +33,13 @@ SELECT test_ext_node_equal(test_ext_node_make('1234'::oid, 2), -- correct node type, missing field SELECT test_ext_node_get_relid('{EXTENSIBLENODE :extnodename TestExtNode}'); +-- other node types, rejected before reaching their core read routines +SELECT test_ext_node_get_relid('{QUERY}'); +SELECT test_ext_node_copy('{VAR}'); +-- extensible node of another kind +SELECT test_ext_node_get_relid('{EXTENSIBLENODE :extnodename NoSuchExtNode}'); +-- not a node string at all +SELECT test_ext_node_get_relid('42'); -- CustomScan tests CREATE TABLE test_extensible_tbl (id integer, val text); diff --git a/src/test/modules/test_extensible/test_extensible.c b/src/test/modules/test_extensible/test_extensible.c index 2a1344f28d5..8ecb171111e 100644 --- a/src/test/modules/test_extensible/test_extensible.c +++ b/src/test/modules/test_extensible/test_extensible.c @@ -463,22 +463,49 @@ test_get_custom_scan_methods(PG_FUNCTION_ARGS) } /* - * Decode a TestExtNode via stringToNode(), rejecting a string describing - * some other kind of node instead of misinterpreting it as one of ours. + * Check that the next token of the string being read is the expected one. + */ +static bool +test_ext_node_expect_token(ReadNodeContext *ctx, const char *expected) +{ + int length; + const char *token = pg_strtok(ctx, &length); + + return token != NULL && length == (int) strlen(expected) && + memcmp(token, expected, length) == 0; +} + +/* + * Decode a TestExtNode via stringToNode(). + * + * stringToNode() assumes its input to be valid, and the read routines of + * readfuncs.c it dispatches to do not check for missing tokens, so handing + * it a string naming any other node type could crash the backend. Hence, + * look at the leading tokens first and refuse anything that would not end + * up in our own nodeRead callback, the only one hardened against arbitrary + * input. */ static TestExtNode * text_to_test_ext_node(text *txt) { - Node *node = stringToNode(text_to_cstring(txt)); + char *str = text_to_cstring(txt); + ReadNodeContext ctx = {.str = str}; + TestExtNode *tnode; - if (node == NULL || !IsA(node, ExtensibleNode) || - strcmp(((ExtensibleNode *) node)->extnodename, TEST_EXT_NODE_NAME) != 0) + if (!test_ext_node_expect_token(&ctx, "{") || + !test_ext_node_expect_token(&ctx, "EXTENSIBLENODE") || + !test_ext_node_expect_token(&ctx, ":extnodename") || + !test_ext_node_expect_token(&ctx, TEST_EXT_NODE_NAME)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("argument is not a serialized \"%s\"", TEST_EXT_NODE_NAME))); - return (TestExtNode *) node; + tnode = (TestExtNode *) stringToNode(str); + Assert(IsA(tnode, ExtensibleNode) && + strcmp(tnode->base.extnodename, TEST_EXT_NODE_NAME) == 0); + + return tnode; } /* -- 2.55.0