Wireshark-dev: Re: [Wireshark-dev] PSA: QString.toUtf8().constData() pattern is unsafe
On Friday 28 November 2014 15:48:29 Evan Huus wrote:
> I'm a bit confused - wouldn't the new instance of QByteArray simply be
> leaked in the example code, as opposed to destructed? C++ doesn't have
> automatic garbage collection...
Why would it be leaked? There is a new QByteArray instance text_utf8
that stays allocated until the scope is left, then its distructor is
called.
Maybe you are confused with the "new" keyword which would require you to
add "delete" to destruct an object?
Peter
> On Fri, Nov 28, 2014 at 2:13 PM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:
> > Hi all,
> >
> > I mostly use Wireshark GTK, but just tried the Qt UI again. A recurring
> > problem was an ASAN crash on shutdown. It turns out that there are many
> > users of this pattern:
> >
> > recent_add_cfilter(NULL, currentText().toUtf8().constData());
> >
> > This is unsafe as currentText().toUtf8() returns a new instance of
> > QByteArray and constData() returns a pointer to data inside that object.
> > After returning, the data is destructed and a use-after-free condition
> > occurs.
> >
> > The more correct way to do this is to use another variable to ensure
> > that a reference is held to that QByteArray:
> >
> > QByteArray text_utf8 = currentText().toUtf8();
> > recent_add_cfilter(NULL, text_utf8.constData());
> >
> > See also the commit message at https://code.wireshark.org/review/5528/
> > Please avoid this pattern in the future, and watch it during reviews.
> > --
> > Kind regards,
> > Peter
> > https://lekensteyn.nl
