[binutils-gdb] [gdb] Fix heap-buffer-overflow in find_program_interpreter

[binutils-gdb] [gdb] Fix heap-buffer-overflow in find_program_interpreter

[Tom de Vries]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8e94bb3e3a478544c0d8abfad8404af015f7130b

commit 8e94bb3e3a478544c0d8abfad8404af015f7130b
Author: Tom de Vries <tdevries@suse.de>
Date:   Fri Oct 14 21:22:57 2022 +0200

    [gdb] Fix heap-buffer-overflow in find_program_interpreter
    
    With the test-case included in this patch, we run into:
    ...
    (gdb) target remote localhost:2347^M
    `target:twice-connect' has disappeared; keeping its symbols.^M
    Remote debugging using localhost:2347^M
    warning: Unable to find dynamic linker breakpoint function.^M
    GDB will be unable to debug shared library initializers^M
    and track explicitly loaded dynamic code.^M
    Reading /usr/lib/debug/.build-id/$hex/$hex.debug from remote target...^M
    0x00007ffff7dd4550 in ?? ()^M
    (gdb) PASS: gdb.server/twice-connect.exp: session=second: gdbserver started
    FAIL: gdb.server/twice-connect.exp: found interpreter
    ...
    
    The problem originates in find_program_interpreter, where
    bfd_get_section_contents is called to read .interp, but fails.  The function
    returns false but the result is ignored, so find_program_interpreter returns
    some random string.
    
    Fix this by checking the result of the call to bfd_get_section_contents.
    
    Tested on x86_64-linux.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29652

Diff:
---
 gdb/solib-svr4.c                           |  8 ++++--
 gdb/testsuite/gdb.server/twice-connect.c   | 22 ++++++++++++++
 gdb/testsuite/gdb.server/twice-connect.exp | 46 ++++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdbserver-support.exp    |  7 ++++-
 4 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 27267e0bde9..7e83819a03d 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -568,9 +568,11 @@ find_program_interpreter (void)
 	int sect_size = bfd_section_size (interp_sect);
 
 	gdb::byte_vector buf (sect_size);
-	bfd_get_section_contents (current_program_space->exec_bfd (),
-				  interp_sect, buf.data (), 0, sect_size);
-	return buf;
+	bool res
+	  = bfd_get_section_contents (current_program_space->exec_bfd (),
+				      interp_sect, buf.data (), 0, sect_size);
+	if (res)
+	  return buf;
       }
    }
 
diff --git a/gdb/testsuite/gdb.server/twice-connect.c b/gdb/testsuite/gdb.server/twice-connect.c
new file mode 100644
index 00000000000..6b3984dc7d2
--- /dev/null
+++ b/gdb/testsuite/gdb.server/twice-connect.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/twice-connect.exp b/gdb/testsuite/gdb.server/twice-connect.exp
new file mode 100644
index 00000000000..c892a0f80a9
--- /dev/null
+++ b/gdb/testsuite/gdb.server/twice-connect.exp
@@ -0,0 +1,46 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if { [skip_gdbserver_tests] } {
+    return 0
+}
+
+if { [build_executable "failed to prepare" $::testfile $::srcfile \
+	  {debug}] } {
+    return -1
+}
+
+# Don't use $binfile arg, to make sure we use the remote file target:$binfile.
+clean_restart
+
+# Start gdbserver, and connect to it, twice.
+foreach_with_prefix session {first second} {
+    lassign [gdbserver_start "" "$binfile"] unused gdbserver_address
+    gdb_test "disconnect"
+    set res [gdb_target_cmd "remote" $gdbserver_address]
+    gdb_assert { $res == 0 } "gdbserver started"
+}
+
+# Verify that we're not running into this warning, which triggers if
+# find_program_interpreter returns something invalid.
+set warning "warning: Unable to find dynamic linker breakpoint function"
+gdb_assert { [regexp $warning $gdb_target_remote_cmd_msg] == 0 } \
+    "found interpreter"
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 08e529fa985..3f2cec246fa 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -48,7 +48,7 @@
 # the connection message in order for the procedure to succeed.
 #
 proc gdb_target_cmd_ext { targetname serialport {additional_text ""} } {
-    global gdb_prompt
+    global gdb_prompt gdb_target_remote_cmd_msg
 
     set serialport_re [string_to_regexp $serialport]
     for {set i 1} {$i <= 3} {incr i} {
@@ -73,22 +73,27 @@ proc gdb_target_cmd_ext { targetname serialport {additional_text ""} } {
 	    }
 	    -re "Remote MIPS debugging.*$additional_text.*$gdb_prompt" {
 		verbose "Set target to $targetname"
+		set gdb_target_remote_cmd_msg $expect_out(buffer)
 		return 0
 	    }
 	    -re "Remote debugging using .*$serialport_re.*$additional_text.*$gdb_prompt $" {
 		verbose "Set target to $targetname"
+		set gdb_target_remote_cmd_msg $expect_out(buffer)
 		return 0
 	    }
 	    -re "Remote debugging using stdio.*$additional_text.*$gdb_prompt $" {
 		verbose "Set target to $targetname"
+		set gdb_target_remote_cmd_msg $expect_out(buffer)
 		return 0
 	    }
 	    -re "Remote target $targetname connected to.*$additional_text.*$gdb_prompt $" {
 		verbose "Set target to $targetname"
+		set gdb_target_remote_cmd_msg $expect_out(buffer)
 		return 0
 	    }
 	    -re "Connected to.*$additional_text.*$gdb_prompt $" {
 		verbose "Set target to $targetname"
+		set gdb_target_remote_cmd_msg $expect_out(buffer)
 		return 0
 	    }
 	    -re "Ending remote.*$gdb_prompt $" { }