[PATCH v1] Fix out-of-bounds access in pg_bsd_indent's parser stack
parser_state stores parser state in p_stack, il, and cstk, which
share the tos index. The existing overflow check used p_stack, the
largest array, allowing out-of-bounds accesses to cstk and il. Size
all three arrays equally.
Also widen the overflow check by one element: the lbrace case pushes
two entries (lbrace and then stmt), so there must be room for two
more elements when parse() is entered.
---
src/tools/pg_bsd_indent/indent_globs.h | 4 ++--
src/tools/pg_bsd_indent/parse.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/tools/pg_bsd_indent/indent_globs.h b/src/tools/pg_bsd_indent/indent_globs.h
index 917961bd3d6..e88dd52c891 100644
--- a/src/tools/pg_bsd_indent/indent_globs.h
+++ b/src/tools/pg_bsd_indent/indent_globs.h
@@ -234,8 +234,8 @@ extern int ifdef_level;
struct parser_state {
int last_token;
int p_stack[256]; /* this is the parsers stack */
- int il[64]; /* this stack stores indentation levels */
- float cstk[32]; /* used to store case stmt indentation levels */
+ int il[256]; /* this stack stores indentation levels */
+ float cstk[256]; /* used to store case stmt indentation levels */
int box_com; /* set to true when we are in a "boxed"
* comment. In that case, the first non-blank
* char should be lined up with the / in / followed by * */
diff --git a/src/tools/pg_bsd_indent/parse.c b/src/tools/pg_bsd_indent/parse.c
index 94cea724393..f50a03193e1 100644
--- a/src/tools/pg_bsd_indent/parse.c
+++ b/src/tools/pg_bsd_indent/parse.c
@@ -203,7 +203,7 @@ parse(int tk) /* tk: the code for the construct scanned */} /* end of switch */
- if (ps.tos >= nitems(ps.p_stack) - 1)
+ if (ps.tos >= nitems(ps.p_stack) - 2)
errx(1, "Parser stack overflow"); reduce(); /* see if any reduction can be done */
--
2.43.0
On 30 Sep 2026, at 12:51, Vadim Shakirov <vadimsakirov5@gmail.com> wrote:
parser_state stores parser state in p_stack, il, and cstk, which
share the tos index. The existing overflow check used p_stack, the
largest array, allowing out-of-bounds accesses to cstk and il. Size
all three arrays equally.Also widen the overflow check by one element: the lbrace case pushes
two entries (lbrace and then stmt), so there must be room for two
more elements when parse() is entered.
I haven't read the patch, but reproducers of bugs are good to have. Is there a
pattern which hits this out-of-bounds access?
--
Daniel Gustafsson
On 2026-Sep-30, Daniel Gustafsson wrote:
I haven't read the patch, but reproducers of bugs are good to have. Is there a
pattern which hits this out-of-bounds access?
Fun. I did this
(echo "int main() {" ; for i in `seq 1 256`; do echo "if ($i) "; done ; echo "switch (argc) {case 1: break;}}" ) > pgindent.c
which produced a smallish file:
$ ls -l pgindent.c
-rw-rw-r-- 1 alvherre alvherre 2497 Sep 30 13:52 pgindent.c
then ran pg_bsd_indent on it, which caused a segmentation fault and
ended with a somewhat larger file
$ ls -l pgindent.c
-rw-rw-r-- 1 alvherre alvherre 599314432 Sep 30 13:53 pgindent.c
That size was the entirety of the free disk space in that partition.
After applying this fix, it doesn't crash anymore, and merely dies with
a "Parser stack overflow" after having written up to "if (251)".
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
“Cuando no hay humildad las personas se degradan” (A. Christie)
On 30 Sep 2026, at 13:59, Álvaro Herrera <alvherre@kurilemu.de> wrote:
On 2026-Sep-30, Daniel Gustafsson wrote:
I haven't read the patch, but reproducers of bugs are good to have. Is there a
pattern which hits this out-of-bounds access?Fun. I did this
(echo "int main() {" ; for i in `seq 1 256`; do echo "if ($i) "; done ; echo "switch (argc) {case 1: break;}}" ) > pgindent.c
Neat, thanks!
--
Daniel Gustafsson