From f0e2acd3ad23964e247a03b649e0c89ee54dbe27 Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Wed, 8 Jul 2026 09:59:46 +0200 Subject: [PATCH 1/2] PG-2540 Fix bug with overread after comment end in extraction After the end of a comment we skipped one extra byte ahead accidentally causing a potential buffer overread if that comment was at the very end and otherwise potentially missing a comment if we had two comments next to eachother. --- src/pg_stat_monitor.c | 6 ++++-- t/003_settings_pgms_extract_comments.pl | 12 ++++++++++++ .../003_settings_pgms_extract_comments.out | 16 +++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/pg_stat_monitor.c b/src/pg_stat_monitor.c index f85d03b2..262c4942 100644 --- a/src/pg_stat_monitor.c +++ b/src/pg_stat_monitor.c @@ -3220,8 +3220,10 @@ extract_query_comments(const char *query, char *comments, size_t buf_len) q_iter++; } } - - q_iter++; + else + { + q_iter++; + } } comments[curr_len] = '\0'; diff --git a/t/003_settings_pgms_extract_comments.pl b/t/003_settings_pgms_extract_comments.pl index 5fdb7f13..79f0434f 100644 --- a/t/003_settings_pgms_extract_comments.pl +++ b/t/003_settings_pgms_extract_comments.pl @@ -59,6 +59,18 @@ extra_params => [ '-a', '-Pformat=aligned', '-Ptuples_only=off' ]); PGSM::append_to_file($stdout); +($cmdret, $stdout, $stderr) = $node->psql( + 'postgres', + 'SELECT 1 /* a *//* b */, 2, 3', + extra_params => [ '-a', '-Pformat=aligned', '-Ptuples_only=off' ]); +PGSM::append_to_file($stdout); + +($cmdret, $stdout, $stderr) = $node->psql( + 'postgres', + 'SELECT 1, 2, 3, 4 /* x */', + extra_params => [ '-a', '-Pformat=aligned', '-Ptuples_only=off' ]); +PGSM::append_to_file($stdout); + ($cmdret, $stdout, $stderr) = $node->psql( 'postgres', 'SELECT query, comments FROM pg_stat_monitor ORDER BY query COLLATE "C";', diff --git a/t/expected/003_settings_pgms_extract_comments.out b/t/expected/003_settings_pgms_extract_comments.out index 2ddc2f29..7452c9e7 100644 --- a/t/expected/003_settings_pgms_extract_comments.out +++ b/t/expected/003_settings_pgms_extract_comments.out @@ -23,14 +23,28 @@ SELECT 1 /* xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx 1 | 2 (1 row) +SELECT 1 /* a *//* b */, 2, 3 + ?column? | ?column? | ?column? +----------+----------+---------- + 1 | 2 | 3 +(1 row) + +SELECT 1, 2, 3, 4 /* x */ + ?column? | ?column? | ?column? | ?column? +----------+----------+----------+---------- + 1 | 2 | 3 | 4 +(1 row) + SELECT query, comments FROM pg_stat_monitor ORDER BY query COLLATE "C"; query | comments ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + SELECT 1 /* a *//* b */, 2, 3 | /* a */, /* b */ SELECT 1 /* xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx */, 2 | /* xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx SELECT 1 AS num /* First comment */, 'John' AS name /* Second comment*/ | /* First comment */, /* Second comment*/ + SELECT 1, 2, 3, 4 /* x */ | /* x */ SELECT name, setting, unit, context, vartype, source, min_val, max_val, enumvals, boot_val, reset_val, pending_restart FROM pg_settings WHERE name = 'pg_stat_monitor.pgsm_extract_comments' | SELECT pg_stat_monitor_reset() | -(4 rows) +(6 rows) SELECT pg_stat_monitor_reset(); pg_stat_monitor_reset From 607214a04c358c663d2c6261908dfecb915e4fbc Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Fri, 3 Jul 2026 14:17:47 +0200 Subject: [PATCH 2/2] Simplify code for parsing comments The code was hard to read for no obvious gain so this attempts to make the code read more like one would expect. Also remove a redundant comment. --- src/pg_stat_monitor.c | 53 ++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/pg_stat_monitor.c b/src/pg_stat_monitor.c index 262c4942..5a033e25 100644 --- a/src/pg_stat_monitor.c +++ b/src/pg_stat_monitor.c @@ -3167,13 +3167,9 @@ static bool append_comment_char(char *comments, size_t buf_len, size_t *idx, char c) { if (*idx >= buf_len - 1) - { - comments[buf_len - 1] = '\0'; return false; - } - comments[*idx] = c; - (*idx)++; + comments[(*idx)++] = c; return true; } @@ -3182,42 +3178,40 @@ static void extract_query_comments(const char *query, char *comments, size_t buf_len) { size_t curr_len = 0; - const char *q_iter = query; Assert(query != NULL); - while (*q_iter) + for (const char *q_iter = query; *q_iter;) { - /* - * multiline comments, + 1 is safe even if we've reach end of string - */ if (*q_iter == '/' && *(q_iter + 1) == '*') { - if (curr_len != 0) + /* Add separator between comments */ + if (curr_len > 0) { if (!append_comment_char(comments, buf_len, &curr_len, ',')) - return; + goto terminate; if (!append_comment_char(comments, buf_len, &curr_len, ' ')) - return; - } - while (*q_iter && *(q_iter + 1) && (*q_iter != '*' || *(q_iter + 1) != '/')) - { - if (!append_comment_char(comments, buf_len, &curr_len, *q_iter)) - return; - q_iter++; + goto terminate; } - if (*q_iter) - { - if (!append_comment_char(comments, buf_len, &curr_len, *q_iter)) - return; - q_iter++; - } - if (*q_iter) + if (!append_comment_char(comments, buf_len, &curr_len, *(q_iter++))) + goto terminate; + if (!append_comment_char(comments, buf_len, &curr_len, *(q_iter++))) + goto terminate; + + while (*q_iter) { - if (!append_comment_char(comments, buf_len, &curr_len, *q_iter)) - return; - q_iter++; + if (*q_iter == '*' && *(q_iter + 1) == '/') + { + if (!append_comment_char(comments, buf_len, &curr_len, *(q_iter++))) + goto terminate; + if (!append_comment_char(comments, buf_len, &curr_len, *(q_iter++))) + goto terminate; + break; + } + + if (!append_comment_char(comments, buf_len, &curr_len, *(q_iter++))) + goto terminate; } } else @@ -3226,6 +3220,7 @@ extract_query_comments(const char *query, char *comments, size_t buf_len) } } +terminate: comments[curr_len] = '\0'; }