[PUSHED] gdb/python: Avoid use after free in py-tui.c

[PUSHED] gdb/python: Avoid use after free in py-tui.c

[Andrew Burgess]

When setting the window title of a tui frame we do this:

  gdb::unique_xmalloc_ptr<char> value
    = python_string_to_host_string (<python-object>);
  ...
  win->window->title = value.get ();

The problem here is that 'get ()' only borrows the pointer from value,
when value goes out of scope the pointer will be freed.  As a result,
the tui frame will be left with a pointer to undefined memory
contents.

Instead we should be using 'value.release ()' to take ownership of the
pointer from value.

gdb/ChangeLog:

	* python/py-tui.c (gdbpy_tui_set_title): Use release, not get, to
	avoid use after free.
---
 gdb/ChangeLog       | 5 +++++
 gdb/python/py-tui.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index ca88f85eb9f..f2c03395a0b 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -433,7 +433,7 @@ gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure)
   if (value == nullptr)
     return -1;
 
-  win->window->title = value.get ();
+  win->window->title = value.release ();
   return 0;
 }
 
-- 
2.25.4

Re: [PUSHED] gdb/python: Avoid use after free in py-tui.c

[Christian Biesinger]

On Fri, Jun 5, 2020 at 1:23 PM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:

> -  win->window->title = value.get ();
> +  win->window->title = value.release ();

Does the old title need to be freed here?

Christian

Re: [PUSHED] gdb/python: Avoid use after free in py-tui.c

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> When setting the window title of a tui frame we do this:
Andrew>   gdb::unique_xmalloc_ptr<char> value
Andrew>     = python_string_to_host_string (<python-object>);
Andrew>   ...
Andrew> win-> window->title = value.get ();

Andrew> The problem here is that 'get ()' only borrows the pointer from value,
Andrew> when value goes out of scope the pointer will be freed.  As a result,
Andrew> the tui frame will be left with a pointer to undefined memory
Andrew> contents.

This does not make sense to me, because tui_win_info::title is a
std::string.

Andrew> Instead we should be using 'value.release ()' to take ownership of the
Andrew> pointer from value.

I suspect this introduces a memory leak instead.

Tom

Re: [PUSHED] gdb/python: Avoid use after free in py-tui.c

[Andrew Burgess]

* Tom Tromey <tom@tromey.com> [2020-06-05 12:37:12 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> When setting the window title of a tui frame we do this:
> Andrew>   gdb::unique_xmalloc_ptr<char> value
> Andrew>     = python_string_to_host_string (<python-object>);
> Andrew>   ...
> Andrew> win-> window->title = value.get ();
> 
> Andrew> The problem here is that 'get ()' only borrows the pointer from value,
> Andrew> when value goes out of scope the pointer will be freed.  As a result,
> Andrew> the tui frame will be left with a pointer to undefined memory
> Andrew> contents.
> 
> This does not make sense to me, because tui_win_info::title is a
> std::string.
> 
> Andrew> Instead we should be using 'value.release ()' to take ownership of the
> Andrew> pointer from value.
> 
> I suspect this introduces a memory leak instead.

My apologies.  I have reverted the commit.

Sorry for the noise.

Thanks,
Andrew