You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
120 lines
4.7 KiB
120 lines
4.7 KiB
From 32df891141689ed73499f4e60f64268957f1e3c2 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
|
|
Date: Wed, 24 Jan 2024 14:03:58 +0100
|
|
Subject: [PATCH] Fix a heap buffer underread in set_buf_break()
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
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
|
|
|