Wireshark mailing list archives

Re: PSA: QString.toUtf8().constData() pattern is unsafe


From: Evan Huus <eapache () gmail com>
Date: Fri, 28 Nov 2014 15:48:29 -0500

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...

Evan

On Fri, Nov 28, 2014 at 2:13 PM, Peter Wu <peter () lekensteyn nl> 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

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe


Current thread: