[PATCH] Fix a heap buffer underread in set_buf_break()

[Petr Písař]

If an opening parenthesis follows a comment with a text, a read from
an invalid address happens in set_buf_break():

    $ printf '/*a*/()' | valgrind -- ./src/indent - -o /dev/null
    ==28887== Memcheck, a memory error detector
    ==28887== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
    ==28887== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
    ==28887== Command: ./src/indent - -o /dev/null
    ==28887==
    ==28887== Invalid read of size 2
    ==28887==    at 0x409989: set_buf_break (output.c:319)
    ==28887==    by 0x401FE7: indent_main_loop (indent.c:640)
    ==28887==    by 0x4022A7: indent (indent.c:759)
    ==28887==    by 0x40294E: indent_single_file (indent.c:1004)
    ==28887==    by 0x402A1C: indent_all (indent.c:1042)
    ==28887==    by 0x402BD0: main (indent.c:1123)
    ==28887==  Address 0x4a5facc is 4 bytes before a block of size 16 alloc'd
    ==28887==    at 0x4849E60: calloc (vg_replace_malloc.c:1595)
    ==28887==    by 0x408B61: xmalloc (globs.c:42)
    ==28887==    by 0x40765E: init_parser (parse.c:73)
    ==28887==    by 0x402B1F: main (indent.c:1101)

It happens when checking an indentation level of the outer scope by indexing
parser_state_tos->paren_indents[]:

    level = parser_state_tos->p_l_follow;
    [...]
    /* Did we just parse a bracket that will be put on the next line
     * by this line break? */
    if ((*token == '(') || (*token == '['))
        --level;    /* then don't take it into account */
    [...]
    if (level == 0) {
    } else {
→       if (parser_state_tos->paren_indents[level - 1] < 0) {...}
    }

The cause is a special case for moving opening parentheses and
brackets to a next line. If parser_state_tos->p_l_follow is zero
(like in the reproducer), the index evaluates to -2 and goes out of
range of the paren_indents array.

This patch simply prevents from decreasing the index under zero when
formating the code. Maybe it leaves some piece of code unformated, but
it's safe.

I checked all places where p_l_follow is set (it is only in
handletoken.c) and they corretly prevent from decrasing it under
zero. That keeps set_buf_break() in output.c as the culprit.

<https://lists.gnu.org/archive/html/bug-indent/2024-01/msg00000.html>

Signed-off-by: Petr Písař <ppisar@redhat.com>
---
 regression/TEST                                     | 2 +-
 regression/input/comment-parent-heap-underread.c    | 3 +++
 regression/standard/comment-parent-heap-underread.c | 5 +++++
 src/output.c                                        | 2 +-
 4 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 regression/input/comment-parent-heap-underread.c
 create mode 100644 regression/standard/comment-parent-heap-underread.c

diff --git a/regression/TEST b/regression/TEST
index 7c07c2e..951b1a2 100755
--- a/regression/TEST
+++ b/regression/TEST
@@ -40,7 +40,7 @@ BUGS="case-label.c one-line-1.c one-line-2.c one-line-3.c \
         macro.c enum.c elif.c nested.c wrapped-string.c minus_predecrement.c \
         bug-gnu-33364.c float-constant-suffix.c block-comments.c \
         no-forced-nl-in-block-init.c hexadecimal_float.c binary-constant.c \
-        comment-heap-overread.c"
+        comment-heap-overread.c comment-parent-heap-underread.c"
 
 INDENTSRC="args.c backup.h backup.c dirent_def.h globs.c indent.h \
         indent.c indent_globs.h io.c lexi.c memcpy.c parse.c pr_comment.c \
diff --git a/regression/input/comment-parent-heap-underread.c 
b/regression/input/comment-parent-heap-underread.c
new file mode 100644
index 0000000..68e13cf
--- /dev/null
+++ b/regression/input/comment-parent-heap-underread.c
@@ -0,0 +1,3 @@
+void foo(void) {
+/*a*/(1);
+}
diff --git a/regression/standard/comment-parent-heap-underread.c 
b/regression/standard/comment-parent-heap-underread.c
new file mode 100644
index 0000000..9a1c6e3
--- /dev/null
+++ b/regression/standard/comment-parent-heap-underread.c
@@ -0,0 +1,5 @@
+void
+foo (void)
+{
+/*a*/ (1);
+}
diff --git a/src/output.c b/src/output.c
index ee01bcc..17eee6e 100644
--- a/src/output.c
+++ b/src/output.c
@@ -290,7 +290,7 @@ void set_buf_break (
     /* Did we just parse a bracket that will be put on the next line
      * by this line break? */
 
-    if ((*token == '(') || (*token == '['))
+    if (level > 0 && ((*token == '(') || (*token == '[')))
     {
         --level;                        /* then don't take it into account */
     }
-- 
2.43.0