[Bug cli/29096] New: AddressSanitizer: heap-use-after-free in execute_command

[Bug cli/29096] New: AddressSanitizer: heap-use-after-free in execute_command

[jan at vrany dot io]

https://sourceware.org/bugzilla/show_bug.cgi?id=29096

            Bug ID: 29096
           Summary: AddressSanitizer: heap-use-after-free in
                    execute_command
           Product: gdb
           Version: HEAD
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: cli
          Assignee: unassigned at sourceware dot org
          Reporter: jan at vrany dot io
  Target Milestone: ---

When I build GDB with ASAN, I observe use-after-free error when executing
Python command that redefines itself:

$ cat /tmp/test.py 
class R(gdb.Command):
        def invoke(self, args, from_tty):
                print("R invoked!")
                R('R', gdb.COMMAND_MAINTENANCE)

R('R', gdb.COMMAND_MAINTENANCE)
jv@sao:~/Projects/gdb/origin_master$ ./gdb/gdb --quiet --data-director
gdb/data-directory/
(gdb) source /tmp/test.py 
(gdb) R
R invoked!
=================================================================
==247122==ERROR: AddressSanitizer: heap-use-after-free on address
0x6120000c5608 at pc 0x5561834259b3 bp 0x7ffea4a37020 sp 0x7ffea4a37018
READ of size 8 at 0x6120000c5608 thread T0
    #0 0x5561834259b2 in execute_cmd_post_hook(cmd_list_element*)
cli/cli-script.c:391
    #1 0x556185201ee2 in execute_command(char const*, int)
/home/jv/Projects/gdb/origin_master/gdb/top.c:704
    #2 0x556183d64935 in command_handler(char const*)
/home/jv/Projects/gdb/origin_master/gdb/event-top.c:598
    #3 0x556183d6590f in command_line_handler(std::unique_ptr<char,
gdb::xfree_deleter<char> >&&)
/home/jv/Projects/gdb/origin_master/gdb/event-top.c:842
    #4 0x556185349bbb in tui_command_line_handler tui/tui-interp.c:278
    #5 0x556183d628d0 in gdb_rl_callback_handler
/home/jv/Projects/gdb/origin_master/gdb/event-top.c:230
    #6 0x556185802e78 in rl_callback_read_char
/home/jv/Projects/gdb/origin_master/readline/readline/callback.c:287
    #7 0x556183d6232d in gdb_rl_callback_read_char_wrapper_noexcept
/home/jv/Projects/gdb/origin_master/gdb/event-top.c:188
    #8 0x556183d62544 in gdb_rl_callback_read_char_wrapper
/home/jv/Projects/gdb/origin_master/gdb/event-top.c:205
    #9 0x556183d63f0c in stdin_event_handler(int, void*)
/home/jv/Projects/gdb/origin_master/gdb/event-top.c:525
    #10 0x556185b61109 in handle_file_event
/home/jv/Projects/gdb/origin_master/gdbsupport/event-loop.cc:574
    #11 0x556185b61a11 in gdb_wait_for_event
/home/jv/Projects/gdb/origin_master/gdbsupport/event-loop.cc:700
    #12 0x556185b5f863 in gdb_do_one_event()
/home/jv/Projects/gdb/origin_master/gdbsupport/event-loop.cc:237
    #13 0x5561843c78ef in start_event_loop
/home/jv/Projects/gdb/origin_master/gdb/main.c:413
    #14 0x5561843c7d0e in captured_command_loop
/home/jv/Projects/gdb/origin_master/gdb/main.c:473
    #15 0x5561843cd025 in captured_main
/home/jv/Projects/gdb/origin_master/gdb/main.c:1335
    #16 0x5561843cd102 in gdb_main(captured_main_args*)
/home/jv/Projects/gdb/origin_master/gdb/main.c:1350
    #17 0x556182a8c0b0 in main /home/jv/Projects/gdb/origin_master/gdb/gdb.c:32
    #18 0x7f3e0d7b07fc in __libc_start_main ../csu/libc-start.c:332
    #19 0x556182a8bea9 in _start
(/home/jv/Projects/gdb/origin_master/gdb/gdb+0xa335ea9)

0x6120000c5608 is located 72 bytes inside of 288-byte region
[0x6120000c55c0,0x6120000c56e0)
freed by thread T0 here:
    #0 0x7f3e0ef9a937 in operator delete(void*)
../../../../src/libsanitizer/asan/asan_new_delete.cpp:160
    #1 0x55618339904f in delete_cmd cli/cli-decode.c:1275
    #2 0x5561833922b0 in do_add_cmd cli/cli-decode.c:189
    #3 0x5561833932d7 in add_cmd(char const*, command_class, char const*,
cmd_list_element**) cli/cli-decode.c:236
    #4 0x5561848acf79 in cmdpy_init python/py-cmd.c:520
    #5 0x7f3e0ea27619 in type_call ../Objects/typeobject.c:1133

previously allocated by thread T0 here:
    #0 0x7f3e0ef99f37 in operator new(unsigned long)
../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x556183392085 in do_add_cmd cli/cli-decode.c:185
    #2 0x5561833932d7 in add_cmd(char const*, command_class, char const*,
cmd_list_element**) cli/cli-decode.c:236
    #3 0x5561848acf79 in cmdpy_init python/py-cmd.c:520
    #4 0x7f3e0ea27619 in type_call ../Objects/typeobject.c:1133

SUMMARY: AddressSanitizer: heap-use-after-free cli/cli-script.c:391 in
execute_cmd_post_hook(cmd_list_element*)
Shadow bytes around the buggy address:
  0x0c2480010a70: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c2480010a80: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c2480010a90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2480010aa0: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c2480010ab0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x0c2480010ac0: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c2480010ad0: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c2480010ae0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c2480010af0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c2480010b00: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
  0x0c2480010b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==247122==ABORTING

Following tentative path seems to fix the problem, but needs more thinking. We
probably also should re-fetch command also after execution pre-hook. 

diff --git a/gdb/top.c b/gdb/top.c
index e776ac2d70e..995b754c2fc 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -623,6 +623,8 @@ execute_command (const char *p, int from_tty)
            }
        }

+      std::string c_name(c->name);
+
       /* If this command has been pre-hooked, run the hook first.  */
       execute_cmd_pre_hook (c);

@@ -662,7 +664,9 @@ execute_command (const char *p, int from_tty)
       maybe_wait_sync_command_done (was_sync);

       /* If this command has been post-hooked, run the hook last.  */
-      execute_cmd_post_hook (c);
+      c = lookup_cmd_exact (c_name.c_str (), cmdlist);
+      if (c != nullptr)
+        execute_cmd_post_hook (c);

       if (repeat_arguments != NULL && cmd_start == saved_command_line)
        {

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug cli/29096] AddressSanitizer: heap-use-after-free in execute_command

[jan at vrany dot io]

https://sourceware.org/bugzilla/show_bug.cgi?id=29096

Jan Vrany <jan at vrany dot io> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jan at vrany dot io

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug cli/29096] AddressSanitizer: heap-use-after-free in execute_command

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29096

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.1
                 CC|                            |vries at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
This seems to have been fixed by this (
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b5661ff24f7111246b9e9b5f1cba5afe9d479daf
) commit:
...
gdb: fix possible use-after-free when executing commands

In principle, `execute_command()` does following:

   struct cmd_list_element *c;
   c = lookup_cmd ( ... );
   ...
   /* If this command has been pre-hooked, run the hook first.  */
   execute_cmd_pre_hook (c);
   ...
   /* ...execute the command `c` ...*/
   ...
   execute_cmd_post_hook (c);

This may lead into use-after-free error.  Imagine the command
being executed is a user-defined Python command that redefines
itself.  In that case, struct `cmd_list_element` pointed to by
`c` is deallocated during its execution so it is no longer valid
when post hook is executed.

To fix this case, this commit looks up the command once again
after it is executed to get pointer to (possibly newly allocated)
`cmd_list_element`.
...

The patch was available in 13.1.

There are two patches fixing fall-out, one of them backported to 13.1, the
other one backported to 13.2.

AFAIU, the correct milestone for this is 13.1.

FWIW, IWBN to also have a test-case for this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.