[PATCH] Avoid buffer overflow in ada_decode

[PATCH] Avoid buffer overflow in ada_decode

[Tom Tromey]

A bug report pointed out a buffer overflow in ada_decode, which Keith
helpfully analyzed.  ada_decode had a logic error when the input was
all digits.  While this isn't valid -- and would probably only appear
in fuzzer tests -- it still should be handled properly.

This patch adds a missing bounds check.  Tested with the self-tests in
an asan build.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30639
---
 gdb/ada-lang.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1261ee8fa05..6e8d98bf4ea 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -57,6 +57,7 @@
 #include "cli/cli-utils.h"
 #include "gdbsupport/function-view.h"
 #include "gdbsupport/byte-vector.h"
+#include "gdbsupport/selftest.h"
 #include <algorithm>
 #include "ada-exp.h"
 #include "charset.h"
@@ -1377,7 +1378,7 @@ ada_decode (const char *encoded, bool wrap, bool operators)
 	i -= 1;
       if (i > 1 && encoded[i] == '_' && encoded[i - 1] == '_')
 	len0 = i - 1;
-      else if (encoded[i] == '$')
+      else if (i >= 0 && encoded[i] == '$')
 	len0 = i;
     }
 
@@ -1574,6 +1575,18 @@ ada_decode (const char *encoded, bool wrap, bool operators)
   return decoded;
 }
 
+#ifdef GDB_SELF_TEST
+
+static void
+ada_decode_tests ()
+{
+  /* This isn't valid, but used to cause a crash.  PR gdb/30639.  The
+     result does not really matter very much.  */
+  SELF_CHECK (ada_decode ("44") == "44");
+}
+
+#endif
+
 /* Table for keeping permanent unique copies of decoded names.  Once
    allocated, names in this table are never released.  While this is a
    storage leak, it should not be significant unless there are massive
@@ -13977,4 +13990,8 @@ DWARF attribute."),
   gdb::observers::new_objfile.attach (ada_new_objfile_observer, "ada-lang");
   gdb::observers::free_objfile.attach (ada_free_objfile_observer, "ada-lang");
   gdb::observers::inferior_exit.attach (ada_inferior_exit, "ada-lang");
+
+#ifdef GDB_SELF_TEST
+  selftests::register_test ("ada-decode", ada_decode_tests);
+#endif
 }
-- 
2.40.1

Re: [PATCH] Avoid buffer overflow in ada_decode

[Keith Seitz]

On 8/16/23 10:31, Tom Tromey via Gdb-patches wrote:
> A bug report pointed out a buffer overflow in ada_decode, which Keith
> helpfully analyzed.  ada_decode had a logic error when the input was
> all digits.  While this isn't valid -- and would probably only appear
> in fuzzer tests -- it still should be handled properly.
> 
> This patch adds a missing bounds check.  Tested with the self-tests in
> an asan build.

That's excellent. Thank you for the quick turnaround!

Reviewed-by: Keith Seitz <keiths@redhat.com>

Keith

Re: [PATCH] Avoid buffer overflow in ada_decode

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> On 8/16/23 10:31, Tom Tromey via Gdb-patches wrote:
>> A bug report pointed out a buffer overflow in ada_decode, which Keith
>> helpfully analyzed.  ada_decode had a logic error when the input was
>> all digits.  While this isn't valid -- and would probably only appear
>> in fuzzer tests -- it still should be handled properly.
>> This patch adds a missing bounds check.  Tested with the self-tests
>> in
>> an asan build.

Keith> That's excellent. Thank you for the quick turnaround!

Keith> Reviewed-by: Keith Seitz <keiths@redhat.com>

Thanks.  I'm going to check this in.

Tom