Re: [PATCH] [gdb] Fix heap-buffer-overflow in args_complete_p

Andrew Burgess
2026-01-05T20:03:53Z

Andrew Burgess <aburgess@redhat.com> writes:

> Tom de Vries <tdevries@suse.de> writes:
>
>> PR gdb/33754 reports a heap-buffer-overflow here in args_complete_p:
>> ...
>> while (*input != '\0')
>> ...
>>
>> Fix this by introducing a lambda function at that safely handles all char
>> array accesses.
>
> Sorry to be a bore, but after reading this commit, and the bug report,
> it's still not obvious to me where the overflow actually occurs.
>
> I totally accept that this code is broken, but as I introduced this bug,
> I wanted to learn from this mistake, but this commit doesn't really
> explain what mistake is being fixed.
>
> Do you think you could explain what's actually going wrong here?

Literally after hitting send, it occurred to me, is the problem maybe
these two lines:

if (*input == '\\' && strchr ("\"\\", *(input + 1)) != nullptr)
++input;

And the other one in the 'else' block? I think if *(input + 1) is '\0',
then the strchr call will return non-nullptr, which wasn't the desired
behaviour, and could result in stepping outside the string.

If this is the case then I think the correct fix would be checking if
the character at 'input + 1' is NULL or not, see the possible patch
below (there's no commit message or anything for it yet).

Thanks,
Andrew

---

diff --git i/gdb/infcmd.c w/gdb/infcmd.c
index 875bbe1ee69..7dd3392c96a 100644
--- i/gdb/infcmd.c
+++ w/gdb/infcmd.c
@@ -148,7 +148,9 @@ args_complete_p (const std::string &args)
and we don't skip the entire '\\' then we'll only skip the
first '\', in which case we might see the second '\' as a '\"'
sequence, which would be wrong. */
- if (*input == '\\' && strchr ("\"\\", *(input + 1)) != nullptr)
+ if (*input == '\\'
+ && *(input + 1) != '\0'
+ && strchr ("\"\\", *(input + 1)) != nullptr)
++input;
/* Otherwise, just look for the closing double quote. */
else if (*input == '"')
@@ -162,7 +164,9 @@ args_complete_p (const std::string &args)
a quoted argument. The '\\' we need to skip so we don't just
skip the first '\' and then incorrectly consider the second
'\' are part of a '\"' or '\'' sequence. */
- if (*input == '\\' && strchr ("\"\\'", *(input + 1)) != nullptr)
+ if (*input == '\\'
+ && *(input + 1) != '\0'
+ && strchr ("\"\\'", *(input + 1)) != nullptr)
++input;
/* Otherwise, check for the start of a single or double quoted
argument. Single quotes have no special meaning on Windows