Skip to content

Make check_stack_depth() effective for embedders; guard recursive output paths (17) #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pyramation wants to merge 4 commits into
base: 17-constructive
Choose a base branch
from
+309 −7
Open

Make check_stack_depth() effective for embedders; guard recursive output paths (17) #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5691182
Make check_stack_depth() effective for embedders and guard the recurs…
pyramation Oct 24, 2026
ab619b8
Free the PL/pgSQL alias list after each parse
pyramation Oct 24, 2026
b5c9e69
fix: return stack-depth errors instead of exiting the process
pyramation Oct 28, 2026
a09aa96
test(stack_depth): avoid stpcpy (MinGW); accept the unguarded protobu…
pyramation Oct 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ examples/normalize_error: examples/normalize_error.c $(ARLIB)
examples/simple_plpgsql: examples/simple_plpgsql.c $(ARLIB)
$(CC) $(TEST_CFLAGS) -o $@ -g examples/simple_plpgsql.c $(ARLIB) $(TEST_LDFLAGS)

TESTS = test/complex test/concurrency test/deparse test/fingerprint test/fingerprint_opts test/is_utility_stmt test/normalize test/normalize_utility test/parse test/parse_opts test/parse_protobuf test/parse_protobuf_opts test/parse_plpgsql test/scan test/split test/summary test/summary_truncate
TESTS = test/complex test/concurrency test/deparse test/fingerprint test/fingerprint_opts test/is_utility_stmt test/normalize test/normalize_utility test/parse test/parse_opts test/parse_protobuf test/parse_protobuf_opts test/parse_plpgsql test/scan test/split test/stack_depth test/summary test/summary_truncate
test: $(TESTS)
ifeq ($(VALGRIND),1)
$(VALGRIND_MEMCHECK) test/complex || (cat test/valgrind.log && false)
Expand All @@ -254,6 +254,7 @@ ifeq ($(VALGRIND),1)
$(VALGRIND_MEMCHECK) test/parse_protobuf_opts || (cat test/valgrind.log && false)
$(VALGRIND_MEMCHECK) test/scan || (cat test/valgrind.log && false)
$(VALGRIND_MEMCHECK) test/split || (cat test/valgrind.log && false)
$(VALGRIND_MEMCHECK) test/stack_depth || (cat test/valgrind.log && false)
$(VALGRIND_MEMCHECK) test/summary || (cat test/valgrind.log && false)
$(VALGRIND_MEMCHECK) test/summary_truncate || (cat test/valgrind.log && false)
# Output-based tests
Expand All @@ -274,6 +275,7 @@ else
test/parse_protobuf_opts
test/scan
test/split
test/stack_depth
test/summary
test/summary_truncate
# Output-based tests
Expand Down Expand Up @@ -335,6 +337,9 @@ test/parse_protobuf_opts: test/parse_protobuf_opts.c test/parse_opts_tests.c $(A
test/scan: test/scan.c test/scan_tests.c $(ARLIB)
$(CC) $(TEST_CFLAGS) -o $@ test/scan.c $(ARLIB) $(TEST_LDFLAGS)

test/stack_depth: test/stack_depth.c $(ARLIB)
$(CC) $(TEST_CFLAGS) -o $@ test/stack_depth.c $(ARLIB) $(TEST_LDFLAGS)

test/split: test/split.c test/split_tests.c $(ARLIB)
$(CC) $(TEST_CFLAGS) -o $@ test/split.c $(ARLIB) $(TEST_LDFLAGS)

Expand Down
5 changes: 5 additions & 0 deletions pg_query.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ void pg_query_free_summary_parse_result(PgQuerySummaryParseResult result);
// Optional, cleans up the top-level memory context (automatically done for threads that exit)
void pg_query_exit(void);

// Optional, makes deeply nested input fail with "stack depth limit exceeded"
// instead of overflowing the native stack. Allows `bytes` of stack below the
// caller's frame, so call it from an outer frame. The setting is per thread.
void pg_query_set_stack_limit(size_t bytes);

// Postgres version information
#define PG_MAJORVERSION "17"
#define PG_VERSION "17.7"
Expand Down
61 changes: 60 additions & 1 deletion src/pg_query_fingerprint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "pg_query_fingerprint.h"

#include "postgres.h"
#include "miscadmin.h"
#define XXH_STATIC_LINKING_ONLY
#include "xxhash/xxhash.h"
#include "lib/ilist.h"

Expand Down Expand Up @@ -225,6 +227,33 @@ _fingerprintList(FingerprintContext *ctx, const List *node, const void *parent,
}
}

/*
* Hash states come from the current memory context rather than malloc, so an
* error thrown mid-fingerprint (e.g. stack depth exceeded) cannot leak them.
* Keeps XXH3's 64-byte alignment the same way XXH_alignedMalloc does.
*/
static XXH3_state_t *
_fingerprintCreateState(void)
{
char *base = palloc0(sizeof(XXH3_state_t) + 64);
size_t offset = 64 - ((uintptr_t) base & 63);
char *ptr = base + offset;

ptr[-1] = (char) offset;
return (XXH3_state_t *) ptr;
}

static void
_fingerprintFreeState(XXH3_state_t *state)
{
char *ptr = (char *) state;

pfree(ptr - (unsigned char) ptr[-1]);
}

#define XXH3_createState _fingerprintCreateState
#define XXH3_freeState _fingerprintFreeState

static void
_fingerprintInitContext(FingerprintContext *ctx, FingerprintContext *parent, bool write_tokens)
{
Expand Down Expand Up @@ -263,6 +292,7 @@ _fingerprintFreeContext(FingerprintContext *ctx) {
void
_fingerprintNode(FingerprintContext *ctx, const void *obj, const void *parent, char *field_name, unsigned int depth)
{
check_stack_depth();
// Some queries are overly complex in their parsetree - lets consistently cut them off at 100 nodes deep
if (depth >= 100) {
return;
Expand Down Expand Up @@ -327,10 +357,12 @@ uint64_t pg_query_fingerprint_node(const void *node)
PgQueryFingerprintResult pg_query_fingerprint_with_opts(const char* input, int parser_options, bool printTokens)
{
MemoryContext ctx = NULL;
MemoryContext mem_ctx;
PgQueryInternalParsetreeAndError parsetree_and_error;
PgQueryFingerprintResult result = {0};

ctx = pg_query_enter_memory_context();
mem_ctx = ctx;

parsetree_and_error = pg_query_raw_parse(input, parser_options);

Expand All @@ -345,7 +377,34 @@ PgQueryFingerprintResult pg_query_fingerprint_with_opts(const char* input, int p
_fingerprintInitContext(&ctx, NULL, printTokens);

if (parsetree_and_error.tree != NULL) {
_fingerprintNode(&ctx, parsetree_and_error.tree, NULL, NULL, 0);
PG_TRY();
{
_fingerprintNode(&ctx, parsetree_and_error.tree, NULL, NULL, 0);
}
PG_CATCH();
{
ErrorData* error_data;
PgQueryError* error;

MemoryContextSwitchTo(mem_ctx);
error_data = CopyErrorData();
error = malloc(sizeof(PgQueryError));
error->message = strdup(error_data->message);
error->filename = strdup(error_data->filename);
error->funcname = strdup(error_data->funcname);
error->context = NULL;
error->lineno = error_data->lineno;
error->cursorpos = error_data->cursorpos;
result.error = error;
FlushErrorState();
}
PG_END_TRY();

if (result.error != NULL) {
_fingerprintFreeContext(&ctx);
pg_query_exit_memory_context(mem_ctx);
return result;
}
}

if (printTokens) {
Expand Down
7 changes: 7 additions & 0 deletions src/pg_query_json_plpgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#include "pg_query.h"
#include "postgres.h"
#include "miscadmin.h"
#include "pg_query_json_plpgsql.h"

#include "pg_query_json_helper.c"
Expand Down Expand Up @@ -138,6 +140,7 @@ static void dump_type(StringInfo out, PLpgSQL_type *node);
static void
dump_stmt(StringInfo out, PLpgSQL_stmt *node)
{
check_stack_depth();

@tenki-reviewer tenki-reviewer Bot Oct 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 bug · high

PL/pgSQL JSON dumper's check_stack_depth throw has no handler

dump_stmt and dump_expr in src/pg_query_json_plpgsql.c:143 now call check_stack_depth(), which throws an ereport(ERROR) when nesting exceeds the stack-depth limit. The caller chain pg_query_parse_plpgsql → plpgsqlToJSON (src/pg_query_parse_plpgsql.c:492) has no PG_TRY — the enclosing PG_TRY at line 345 only covers compilation and has already exited by the time the dump runs. With no active exception handler, pg_re_throw promotes the error to FATAL and exits the host process instead of returning a PgQueryError.

📋 Prompt for AI Agents

In src/pg_query_parse_plpgsql.c, inside the loop of pg_query_parse_plpgsql (around lines 487-508), wrap func_json = plpgsqlToJSON(func_and_error.func); in a PG_TRY/PG_CATCH. In the PG_CATCH, MemoryContextSwitchTo the outer context ctx, CopyErrorData, build a malloc'd PgQueryError with strdup'd message/filename/funcname and NULL context, assign it to result.error, and FlushErrorState — mirroring the existing PG_CATCH at lines 369-388 — then skip appending the function and break out of the loop. This is needed because dump_stmt and dump_expr in src/pg_query_json_plpgsql.c now call check_stack_depth(), which throws an ereport(ERROR); with no handler the error becomes FATAL and exits the process on deeply nested PL/pgSQL input.

appendStringInfoChar(out, '{');
switch (node->cmd_type)
{
Expand Down Expand Up @@ -625,6 +628,7 @@ dump_getdiag_item(StringInfo out, PLpgSQL_diag_item *node)
static void
dump_expr(StringInfo out, PLpgSQL_expr *node)
{
check_stack_depth();
WRITE_NODE_TYPE("PLpgSQL_expr");

WRITE_STRING_FIELD(query, query, query);
Expand Down Expand Up @@ -806,7 +810,10 @@ pg_query_plpgsql_reset_aliases(void)
int i;
for (i = 0; i < plpgsql_naliases; i++)
free(plpgsql_aliases[i].name);
free(plpgsql_aliases);
plpgsql_aliases = NULL;
plpgsql_naliases = 0;
plpgsql_aliases_size = 0;
}

void
Expand Down
2 changes: 2 additions & 0 deletions src/pg_query_outfuncs_json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "pg_query_outfuncs.h"

#include "postgres.h"
#include "miscadmin.h"

#include <ctype.h>

Expand Down Expand Up @@ -282,6 +283,7 @@ _outAConst(StringInfo out, const A_Const *node)
static void
_outNode(StringInfo out, const void *obj)
{
check_stack_depth();
if (obj == NULL)
{
appendStringInfoString(out, "null");
Expand Down
2 changes: 2 additions & 0 deletions src/pg_query_outfuncs_protobuf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "pg_query_outfuncs.h"

#include "postgres.h"
#include "miscadmin.h"
#include <ctype.h>
#include "access/relation.h"
#include "nodes/parsenodes.h"
Expand Down Expand Up @@ -246,6 +247,7 @@ _outAConst(PgQuery__AConst* out, const A_Const *node)
static void
_outNode(PgQuery__Node* out, const void *obj)
{
check_stack_depth();
if (obj == NULL)
return; // Keep out as NULL

Expand Down
59 changes: 55 additions & 4 deletions src/pg_query_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,39 @@ PgQueryParseResult pg_query_parse_opts(const char* input, int parser_options)
result.stderr_buffer = parsetree_and_error.stderr_buffer;
result.error = parsetree_and_error.error;

tree_json = pg_query_nodes_to_json(parsetree_and_error.tree);
result.parse_tree = strdup(tree_json);
pfree(tree_json);
if (result.error == NULL)
{
PG_TRY();
{
tree_json = pg_query_nodes_to_json(parsetree_and_error.tree);
result.parse_tree = strdup(tree_json);
pfree(tree_json);
}
PG_CATCH();
{
ErrorData* error_data;
PgQueryError* error;

MemoryContextSwitchTo(ctx);
error_data = CopyErrorData();
error = malloc(sizeof(PgQueryError));
error->message = strdup(error_data->message);
error->filename = strdup(error_data->filename);
error->funcname = strdup(error_data->funcname);
error->context = NULL;
error->lineno = error_data->lineno;
error->cursorpos = error_data->cursorpos;
result.error = error;
FlushErrorState();
}
PG_END_TRY();
}
else
{
tree_json = pg_query_nodes_to_json(parsetree_and_error.tree);
result.parse_tree = strdup(tree_json);
pfree(tree_json);
}

pg_query_exit_memory_context(ctx);

Expand All @@ -162,7 +192,28 @@ PgQueryProtobufParseResult pg_query_parse_protobuf_opts(const char* input, int p
// These are all malloc-ed and will survive exiting the memory context, the caller is responsible to free them now
result.stderr_buffer = parsetree_and_error.stderr_buffer;
result.error = parsetree_and_error.error;
result.parse_tree = pg_query_nodes_to_protobuf(parsetree_and_error.tree);
PG_TRY();
{
result.parse_tree = pg_query_nodes_to_protobuf(parsetree_and_error.tree);
}
PG_CATCH();
{
ErrorData* error_data;
PgQueryError* error;

MemoryContextSwitchTo(ctx);
error_data = CopyErrorData();
error = malloc(sizeof(PgQueryError));
error->message = strdup(error_data->message);
error->filename = strdup(error_data->filename);
error->funcname = strdup(error_data->funcname);
error->context = NULL;
error->lineno = error_data->lineno;
error->cursorpos = error_data->cursorpos;
result.error = error;
FlushErrorState();
}
PG_END_TRY();

pg_query_exit_memory_context(ctx);

Expand Down
33 changes: 32 additions & 1 deletion src/pg_query_parse_plpgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,38 @@ PgQueryPlpgsqlParseResult pg_query_parse_plpgsql(const char* input)
char *new_out;
size_t new_out_len;

func_json = plpgsqlToJSON(func_and_error.func);
PG_TRY();
{
func_json = plpgsqlToJSON(func_and_error.func);
}
PG_CATCH();
{
ErrorData* error_data;
PgQueryError* error;

MemoryContextSwitchTo(ctx);
error_data = CopyErrorData();
error = malloc(sizeof(PgQueryError));
error->message = strdup(error_data->message);
error->filename = strdup(error_data->filename);
error->funcname = strdup(error_data->funcname);
error->context = NULL;
error->lineno = error_data->lineno;
error->cursorpos = error_data->cursorpos;
result.error = error;
FlushErrorState();
}
PG_END_TRY();

plpgsql_free_function_memory(func_and_error.func);

if (result.error != NULL) {
free(parse_result.stderr_buffer);
pg_query_plpgsql_reset_aliases();
pg_query_exit_memory_context(ctx);
return result;
}

new_out_len = strlen(result.plpgsql_funcs) + strlen(func_json) + 3;
new_out = malloc(new_out_len);
int n = snprintf(new_out, new_out_len, "%s%s,\n", result.plpgsql_funcs, func_json);
Expand All @@ -512,6 +541,8 @@ PgQueryPlpgsqlParseResult pg_query_parse_plpgsql(const char* input)
result.plpgsql_funcs[strlen(result.plpgsql_funcs) - 1] = ']';

free(parse_result.stderr_buffer);
/* Drop the aliases recorded during this compile so nothing outlives the call */
pg_query_plpgsql_reset_aliases();
pg_query_exit_memory_context(ctx);

return result;
Expand Down
11 changes: 11 additions & 0 deletions src/postgres/src_backend_tcop_postgres.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,3 +826,14 @@ stack_is_too_deep(void)
* Disable statement timeout, if active.
*/


/* libpg-query-node: set the stack-depth reference point and limit from the embedder. */
void
pg_query_set_stack_limit(size_t bytes)
{
char here;

stack_base_ptr = &here;
max_stack_depth_bytes = (long) bytes;
max_stack_depth = (int) (bytes / 1024);
}
Comment on lines +832 to +839

@tenki-reviewer tenki-reviewer Bot Oct 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bug · medium

pg_query_set_stack_limit lacks validation, declaration, per-thread docs

The new exported pg_query_set_stack_limit accepts any size_t without validation (src/postgres/src_backend_tcop_postgres.c:832): with bytes == 0 it sets a non-NULL stack_base_ptr and a zero limit, so stack_is_too_deep() returns true immediately and every subsequent recursive call — even the shallowest query — fails with "stack depth limit exceeded"; with very large values, (int)(bytes / 1024) and (long) bytes overflow or wrap, silently disabling protection. The function is also not declared in the public header pg_query.h, so embedders have no ABI contract, and since all three state variables are __thread, the setting applies only to the calling thread — an embedder configuring from a setup thread gets silently different behavior on worker threads. Impact: an embedder passing an unvalidated config value gets spurious errors or silently no stack protection, with no documented contract to guide correct use.

📋 Prompt for AI Agents
  1. In pg_query.h, add a declaration void pg_query_set_stack_limit(size_t bytes); near the other public entry points (around line 119), with a comment explaining that the stack base and limit are thread-local (__thread state in src/postgres/src_backend_tcop_postgres.c), so the function must be called on every thread that will run pg_query parsing, and that stack-depth checks remain inert until it is called. 2) In src/postgres/src_backend_tcop_postgres.c lines 832-839, guard the body: if bytes is below a safe minimum (e.g. 64 * 1024), either ignore the call or clamp to the existing default (100 * 1024) so that bytes = 0 cannot make stack_is_too_deep return true for every call, and clamp large values to (size_t)INT_MAX * 1024 so the int/long casts cannot overflow.

2 changes: 2 additions & 0 deletions src/postgres_deparse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "postgres_deparse.h"

#include "postgres.h"
#include "miscadmin.h"
#include "catalog/index.h"
#include "catalog/pg_am.h"
#include "catalog/pg_attribute.h"
Expand Down Expand Up @@ -783,6 +784,7 @@ static void deparseCExpr(DeparseState *state, Node *node);
// "a_expr" in gram.y
static void deparseExpr(DeparseState *state, Node *node, DeparseNodeContext context)
{
check_stack_depth();

@tenki-reviewer tenki-reviewer Bot Oct 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bug · medium

Statement-level deparse recursion bypasses deparseExpr guard

The PR's check_stack_depth() guard was added only at deparseExpr (src/postgres_deparse.c:787), but statement-level recursion does not funnel through it: deparseSubLink calls deparseSelectStmt directly (src/postgres_deparse.c:3548), and nested subqueries via deparseRangeSubselect (line 4165), set-operation operands (lines 2849, 2874), and rule/view actions form recursion cycles that never cross the guarded frame. A parse tree with deeply nested subqueries can therefore overflow the real native stack and crash the process with SIGSEGV instead of raising the intended "stack depth limit exceeded" error.

📋 Prompt for AI Agents

In src/postgres_deparse.c, add check_stack_depth(); as the first statement of deparseSelectStmt and deparseStmt — the functions that close statement-level recursion cycles (deparseSubLink at line 3548, deparseRangeSubselect at line 4165, and set-operation recursion at lines 2849/2874 call deparseSelectStmt directly without passing through the deparseExpr guard at line 787). Verify other direct self-recursive cycles (e.g. deparseConstraint, deparseAlterTableCmd via deparseSchemaStmt) are covered by these two insertion points, and add guards to any cycle still not crossing a guarded frame.

if (node == NULL)
return;
switch (nodeTag(node))
Expand Down
Loading
Loading