From 8c91206e6ff880a1243b4dad45d4889c71704e63 Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sun, 21 Jun 2026 12:31:03 -0700 Subject: [PATCH 1/3] Add stack overflow crash protection, error out instead This makes libpg_query return the standard "stack depth limit exceeded" error that Postgres also shows for overly deep queries, instead of crashing when stack depth gets exceeded. This error is triggered by appropriately placed calls to the Postgres check_stack_depth() function, and goes together with initializing the stack base and depth in pg_query_init. The maximum stack depth is handled similar to Postgres max_stack_depth variable, defaulting to 100 kB, and auto-sized up to 2MB. Because we may be called in threads with varying stack sizes, and from different host program locations, the allowed stack depth is recalculated for each call to libpg_query. Per multiple reports from OSS-Fuzz. --- Makefile | 7 +- scripts/extract_source.rb | 2 + src/pg_query.c | 155 ++++++++++++++++++ src/pg_query_outfuncs_json.c | 3 + src/pg_query_outfuncs_protobuf.c | 3 + src/pg_query_parse.c | 79 ++++++++- .../src_backend_utils_misc_stack_depth.c | 26 +++ src/postgres_deparse.c | 3 + test/stack_depth.c | 122 ++++++++++++++ 9 files changed, 395 insertions(+), 5 deletions(-) create mode 100644 test/stack_depth.c diff --git a/Makefile b/Makefile index fdf5e2c2d..4af09440a 100644 --- a/Makefile +++ b/Makefile @@ -238,7 +238,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) @@ -255,6 +255,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 @@ -275,6 +276,7 @@ else test/parse_protobuf_opts test/scan test/split + test/stack_depth test/summary test/summary_truncate # Output-based tests @@ -339,6 +341,9 @@ test/scan: test/scan.c test/scan_tests.c $(ARLIB) test/split: test/split.c test/split_tests.c $(ARLIB) $(CC) $(TEST_CFLAGS) -o $@ test/split.c $(ARLIB) $(TEST_LDFLAGS) +test/stack_depth: test/stack_depth.c $(ARLIB) + $(CC) $(TEST_CFLAGS) -o $@ test/stack_depth.c $(ARLIB) $(TEST_LDFLAGS) + prefix = /usr/local libdir = $(prefix)/lib includedir = $(prefix)/include diff --git a/scripts/extract_source.rb b/scripts/extract_source.rb index 6d66a83cd..898734983 100644 --- a/scripts/extract_source.rb +++ b/scripts/extract_source.rb @@ -674,6 +674,8 @@ def write_out # Other required functions runner.deep_resolve('pg_printf') runner.deep_resolve('pg_strncasecmp') +runner.deep_resolve('set_stack_base') +runner.deep_resolve('assign_max_stack_depth') # Retain these functions for optional 32-bit support # (see BITS_PER_BITMAPWORD checks in bitmapset.c) diff --git a/src/pg_query.c b/src/pg_query.c index 141c4e021..a8a967f88 100644 --- a/src/pg_query.c +++ b/src/pg_query.c @@ -1,14 +1,31 @@ +#if defined(__linux__) && !defined(_GNU_SOURCE) +/* Allow using pthread_getattr_np on glibc/musl. Must precede any includes. */ +#define _GNU_SOURCE +#endif + #include "pg_query.h" #include "pg_query_internal.h" +#include #include #include #include +#include #ifdef HAVE_PTHREAD #include #endif +#if defined(HAVE_PTHREAD) && (defined(__FreeBSD__) || defined(__NetBSD__)) +/* For pthread_attr_get_np() */ +#include +#endif + +#if defined(_WIN32) +/* For GetCurrentThreadStackLimits(); port/win32.h already sets _WIN32_WINNT. */ +#include +#endif + #include const char* progname = "pg_query"; @@ -20,8 +37,34 @@ static pthread_key_t pg_query_thread_exit_key; static void pg_query_thread_exit(void *key); #endif +static __thread char *pg_query_lowest_stackaddr = NULL; +static char *get_lowest_stackaddr(void); +static void set_max_stack_depth(void); + void pg_query_init(void) { + /* + * Set the reference point for stack depth checking, similiar to how + * Postgres does in its main() function. We have to re-do this on every + * execution since we may be called from different parts in our host + * program. + * + * The check_stack_depth() function called by recursive tree walkers + * measures the current stack depth against this base and raises a + * "stack depth limit exceeded" error before we overflow and crash. + */ + set_stack_base(); + + /* + * Size the budget to this thread's actual stack; must follow + * set_stack_base() so both measure from the same point. + */ + set_max_stack_depth(); + + /* + * Later parts of initialization don't have to re-run if we execute again + * in the same thread, since they are not affected by the calling location. + */ if (pg_query_initialized != 0) return; pg_query_initialized = 1; @@ -111,3 +154,115 @@ void pg_query_free_error(PgQueryError *error) free(error); } + +/* + * Get this thread's lowest addressable stack address and cache it for + * subsequent calls. Returns NULL if address can't be determined. + */ +static char *get_lowest_stackaddr(void) +{ + if (pg_query_lowest_stackaddr) + return pg_query_lowest_stackaddr; + +#if defined(_WIN32) + { + ULONG_PTR low, high; + + GetCurrentThreadStackLimits(&low, &high); + + if (low == 0) + return NULL; + + pg_query_lowest_stackaddr = (char *) low; + } +#elif defined(HAVE_PTHREAD) && defined(__linux__) + { + pthread_attr_t attr; + void *stack_addr = NULL; + size_t stack_size = 0; + + if (pthread_getattr_np(pthread_self(), &attr) != 0) + return NULL; + + if (pthread_attr_getstack(&attr, &stack_addr, &stack_size) == 0 && stack_addr != NULL) + pg_query_lowest_stackaddr = (char *) stack_addr; + + pthread_attr_destroy(&attr); + } +#elif defined(HAVE_PTHREAD) && defined(__APPLE__) + { + char *stack_high = (char *) pthread_get_stackaddr_np(pthread_self()); + size_t stack_size = pthread_get_stacksize_np(pthread_self()); + + if (stack_size == 0) + return NULL; + + pg_query_lowest_stackaddr = stack_high - stack_size; + } +#elif defined(HAVE_PTHREAD) && (defined(__FreeBSD__) || defined(__NetBSD__)) + { + pthread_attr_t attr; + void *stack_addr = NULL; + size_t stack_size = 0; + + if (pthread_attr_init(&attr) != 0) + return NULL; + + if (pthread_attr_get_np(pthread_self(), &attr) == 0 && + pthread_attr_getstack(&attr, &stack_addr, &stack_size) == 0 && + stack_addr != NULL) + { + pg_query_lowest_stackaddr = (char *) stack_addr; + } + + pthread_attr_destroy(&attr); + } +#endif + + return pg_query_lowest_stackaddr; +} + +/* + * Raise max_stack_depth to fit the headroom on the calling thread's stack, + * capped at 2MB, similar to how InitializeGUCOptionsFromEnvironment sets + * max_stack_depth. + * + * Re-evaluated for each call, since libpg_query gets embedded into other + * programs, so we have to be conscious of our place in the stack, and measure + * the remaining stack space we can use below the current address. + */ +static void set_max_stack_depth(void) +{ + char stack_here; + char* lowest_stackaddr = get_lowest_stackaddr(); + ssize_t usable_stack_depth; + ssize_t new_limit; + + if (lowest_stackaddr == NULL) + return; + + usable_stack_depth = (ssize_t) (&stack_here - lowest_stackaddr); + if (usable_stack_depth < 0) + return; + + /* + * Determine new limit keeping a similar padding to Postgres via + * STACK_DEPTH_SLOP (512 kB) to account for error handling and places + * where we don't check stack depth frequently. + */ + new_limit = (usable_stack_depth - STACK_DEPTH_SLOP) / 1024; + + /* + * In case the new limit would put us below 100 kB, ignore it, since that + * is our required minimum and the default of the setting. + */ + if (new_limit <= 100) + return; + + /* Clamp at 2MB, like Postgres auto-detection logic. */ + if (new_limit > 2048) + new_limit = 2048; + + max_stack_depth = (int) new_limit; + assign_max_stack_depth((int) new_limit, NULL); +} diff --git a/src/pg_query_outfuncs_json.c b/src/pg_query_outfuncs_json.c index e5025ed6a..e1dacc7a8 100644 --- a/src/pg_query_outfuncs_json.c +++ b/src/pg_query_outfuncs_json.c @@ -9,6 +9,7 @@ #include "nodes/plannodes.h" #include "nodes/value.h" #include "utils/datum.h" +#include "miscadmin.h" #include "pg_query_json_helper.c" @@ -289,6 +290,8 @@ _outAConst(StringInfo out, const A_Const *node) static void _outNode(StringInfo out, const void *obj) { + check_stack_depth(); + if (obj == NULL) { appendStringInfoString(out, "null"); diff --git a/src/pg_query_outfuncs_protobuf.c b/src/pg_query_outfuncs_protobuf.c index 64ccdf1b0..d4eff3324 100644 --- a/src/pg_query_outfuncs_protobuf.c +++ b/src/pg_query_outfuncs_protobuf.c @@ -7,6 +7,7 @@ #include "nodes/plannodes.h" #include "nodes/value.h" #include "utils/datum.h" +#include "miscadmin.h" #include "protobuf/pg_query.pb-c.h" @@ -246,6 +247,8 @@ _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 diff --git a/src/pg_query_parse.c b/src/pg_query_parse.c index 3f2c111ab..59fd40a80 100644 --- a/src/pg_query_parse.c +++ b/src/pg_query_parse.c @@ -135,9 +135,44 @@ 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_query_exit_memory_context(ctx); + return result; + } + + /* + * Serialize the tree to JSON. This walks the parse tree recursively and + * may throw (e.g. "stack depth limit exceeded" for deeply nested + * expressions), so it needs its own error handling. + */ + 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(); + + // Note: This is intentionally malloc so exiting the memory context doesn't free this + 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); @@ -162,7 +197,43 @@ 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); + + if (result.error != NULL) + { + pg_query_exit_memory_context(ctx); + return result; + } + + /* + * Serialize the tree to protobuf. This walks the parse tree recursively + * and may throw (e.g. "stack depth limit exceeded" for deeply nested + * expressions), so it needs its own error handling. + */ + 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(); + + // Note: This is intentionally malloc so exiting the memory context doesn't free this + 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); diff --git a/src/postgres/src_backend_utils_misc_stack_depth.c b/src/postgres/src_backend_utils_misc_stack_depth.c index b1b6690ff..8eb4abf07 100644 --- a/src/postgres/src_backend_utils_misc_stack_depth.c +++ b/src/postgres/src_backend_utils_misc_stack_depth.c @@ -5,6 +5,8 @@ * - max_stack_depth_bytes * - check_stack_depth * - max_stack_depth + * - set_stack_base + * - assign_max_stack_depth *-------------------------------------------------------------------- */ @@ -53,12 +55,30 @@ static __thread char *stack_base_ptr = NULL; * * Returns the old reference point, if any. */ +pg_stack_base_t +set_stack_base(void) +{ #ifndef HAVE__BUILTIN_FRAME_ADDRESS + char stack_base; #endif + pg_stack_base_t old; + + old = stack_base_ptr; + + /* + * Set up reference point for stack depth checking. On recent gcc we use + * __builtin_frame_address() to avoid a warning about storing a local + * variable's address in a long-lived variable. + */ #ifdef HAVE__BUILTIN_FRAME_ADDRESS + stack_base_ptr = __builtin_frame_address(0); #else + stack_base_ptr = &stack_base; #endif + return old; +} + /* * restore_stack_base: restore reference point for stack depth checking * @@ -132,7 +152,13 @@ stack_is_too_deep(void) /* GUC assign hook for max_stack_depth */ +void +assign_max_stack_depth(int newval, void *extra) +{ + ssize_t newval_bytes = newval * (ssize_t) 1024; + max_stack_depth_bytes = newval_bytes; +} /* * Obtain platform stack depth limit (in bytes) diff --git a/src/postgres_deparse.c b/src/postgres_deparse.c index 58401ad36..26baa2119 100644 --- a/src/postgres_deparse.c +++ b/src/postgres_deparse.c @@ -17,6 +17,7 @@ #include "utils/datetime.h" #include "utils/timestamp.h" #include "utils/xml.h" +#include "miscadmin.h" /* * # Deparser overview @@ -784,6 +785,8 @@ static void deparseCExpr(DeparseState *state, Node *node); // "a_expr" in gram.y static void deparseExpr(DeparseState *state, Node *node, DeparseNodeContext context) { + check_stack_depth(); + if (node == NULL) return; switch (nodeTag(node)) diff --git a/test/stack_depth.c b/test/stack_depth.c new file mode 100644 index 000000000..d0a960fb8 --- /dev/null +++ b/test/stack_depth.c @@ -0,0 +1,122 @@ +#include + +#include +#include +#include +#include + +// Regression test for stack overflows when walking deeply nested parse trees. +// +// A deeply nested expression like "SELECT 1%1%1%...%1" parses into a very deep +// A_Expr tree. Recursively serializing or walking that tree (JSON/protobuf +// output, deparsing, normalizing, ...) could overflow the C stack and crash, +// these tests verify we return an error instead, like Postgres does. + +static char *build_deep_query(int depth) +{ + // "SELECT 1" followed by "%1" repeated `depth` times + size_t len = strlen("SELECT 1") + (size_t) depth * 2; + char *query = malloc(len + 1); + char *p = query; + + p += sprintf(p, "SELECT 1"); + for (int i = 0; i < depth; i++) + p += sprintf(p, "%%1"); + + return query; +} + +static bool is_clean(const PgQueryError *error) +{ + if (error == NULL) + return true; + return strstr(error->message, "stack depth limit exceeded") != NULL; +} + +int main() +{ + bool ret_code = 0; + char *query = build_deep_query(100000); + + // JSON output (pg_query_parse) + { + PgQueryParseResult result = pg_query_parse(query); + if (is_clean(result.error)) { + printf("."); + } else { + ret_code = -1; + printf("INVALID parse result, expected clean error, got: %s\n", + result.error ? result.error->message : "(success)"); + } + pg_query_free_parse_result(result); + } + + // Protobuf output (pg_query_parse_protobuf) + { + PgQueryProtobufParseResult result = pg_query_parse_protobuf(query); + if (is_clean(result.error)) { + printf("."); + } else { + ret_code = -1; + printf("INVALID protobuf parse result, expected clean error, got: %s\n", + result.error->message); + } + pg_query_free_protobuf_parse_result(result); + } + + // Normalize (const_record_walker / raw_expression_tree_walker) + { + PgQueryNormalizeResult result = pg_query_normalize(query); + if (is_clean(result.error)) { + printf("."); + } else { + ret_code = -1; + printf("INVALID normalize result, expected clean error, got: %s\n", + result.error->message); + } + pg_query_free_normalize_result(result); + } + + // Fingerprint + { + PgQueryFingerprintResult result = pg_query_fingerprint(query); + if (is_clean(result.error)) { + printf("."); + } else { + ret_code = -1; + printf("INVALID fingerprint result, expected clean error, got: %s\n", + result.error->message); + } + pg_query_free_fingerprint_result(result); + } + + // Deparse (deparseExpr) - exercise via a protobuf round-trip at a depth that + // serializes successfully, so deparsing actually walks a deep tree. + { + char *shallow = build_deep_query(100); + PgQueryProtobufParseResult parsed = pg_query_parse_protobuf(shallow); + if (parsed.error) { + ret_code = -1; + printf("INVALID: could not produce protobuf for deparse test: %s\n", parsed.error->message); + } else { + PgQueryDeparseResult result = pg_query_deparse_protobuf(parsed.parse_tree); + if (is_clean(result.error)) { + printf("."); + } else { + ret_code = -1; + printf("INVALID deparse result, expected clean error, got: %s\n", + result.error->message); + } + pg_query_free_deparse_result(result); + } + pg_query_free_protobuf_parse_result(parsed); + free(shallow); + } + + printf("\n"); + + free(query); + pg_query_exit(); + + return ret_code; +} From 965f3002c3365cdd2292b429eaffa663490321fe Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Sun, 21 Jun 2026 14:18:59 -0700 Subject: [PATCH 2/3] Add check_stack_depth to _outNode for protobuf-cpp variant --- src/pg_query_outfuncs_protobuf_cpp.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pg_query_outfuncs_protobuf_cpp.cc b/src/pg_query_outfuncs_protobuf_cpp.cc index 14bc0e738..9f3a2524d 100644 --- a/src/pg_query_outfuncs_protobuf_cpp.cc +++ b/src/pg_query_outfuncs_protobuf_cpp.cc @@ -19,6 +19,7 @@ extern "C" #include "nodes/plannodes.h" #include "nodes/value.h" #include "utils/datum.h" +#include "miscadmin.h" } #define OUT_TYPE(typename, typename_c) pg_query::typename* @@ -203,6 +204,8 @@ _outAConst(pg_query::A_Const* out_node, const A_Const *node) static void _outNode(pg_query::Node* out, const void *obj) { + check_stack_depth(); + if (obj == NULL) return; // Keep out as NULL From 994fdc926c232f9a884deb653b0bc0bc01598805 Mon Sep 17 00:00:00 2001 From: Lukas Fittl Date: Tue, 23 Jun 2026 12:01:51 -0700 Subject: [PATCH 3/3] Address memory leak by explicitly freeing memory --- src/pg_query_outfuncs_protobuf_cpp.cc | 61 +++++++++++++++++++-------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/pg_query_outfuncs_protobuf_cpp.cc b/src/pg_query_outfuncs_protobuf_cpp.cc index 9f3a2524d..915669f1c 100644 --- a/src/pg_query_outfuncs_protobuf_cpp.cc +++ b/src/pg_query_outfuncs_protobuf_cpp.cc @@ -227,26 +227,38 @@ pg_query_nodes_to_protobuf(const void *obj) { PgQueryProtobuf protobuf; const ListCell *lc; - pg_query::ParseResult parse_result; + if (obj == NULL) { protobuf.data = strdup(""); protobuf.len = 0; return protobuf; } - parse_result.set_version(PG_VERSION_NUM); - foreach(lc, (List*) obj) + pg_query::ParseResult *parse_result = new pg_query::ParseResult(); + + PG_TRY(); { - _outRawStmt(parse_result.add_stmts(), (const RawStmt*) lfirst(lc)); - } + parse_result->set_version(PG_VERSION_NUM); + foreach(lc, (List*) obj) + { + _outRawStmt(parse_result->add_stmts(), (const RawStmt*) lfirst(lc)); + } - std::string output; - parse_result.SerializeToString(&output); + std::string output; + parse_result->SerializeToString(&output); - protobuf.data = (char*) calloc(output.size(), sizeof(char)); - memcpy(protobuf.data, output.data(), output.size()); - protobuf.len = output.size(); + protobuf.data = (char*) calloc(output.size(), sizeof(char)); + memcpy(protobuf.data, output.data(), output.size()); + protobuf.len = output.size(); + } + PG_CATCH(); + { + delete parse_result; + PG_RE_THROW(); + } + PG_END_TRY(); + delete parse_result; return protobuf; } @@ -254,19 +266,32 @@ extern "C" char * pg_query_nodes_to_json(const void *obj) { const ListCell *lc; - pg_query::ParseResult parse_result; + char *result = NULL; if (obj == NULL) return pstrdup("{}"); - parse_result.set_version(PG_VERSION_NUM); - foreach(lc, (List*) obj) + pg_query::ParseResult *parse_result = new pg_query::ParseResult(); + + PG_TRY(); { - _outRawStmt(parse_result.add_stmts(), (const RawStmt*) lfirst(lc)); - } + parse_result->set_version(PG_VERSION_NUM); + foreach(lc, (List*) obj) + { + _outRawStmt(parse_result->add_stmts(), (const RawStmt*) lfirst(lc)); + } - std::string output; - google::protobuf::util::MessageToJsonString(parse_result, &output); + std::string output; + google::protobuf::util::MessageToJsonString(*parse_result, &output); + result = pstrdup(output.c_str()); + } + PG_CATCH(); + { + delete parse_result; + PG_RE_THROW(); + } + PG_END_TRY(); - return pstrdup(output.c_str()); + delete parse_result; + return result; }