[vlc-devel] [PATCH] gui/qt: bookmarks: fix psz_name lifetime

Romain Vimont rom at rom1v.com
Fri Nov 10 16:55:40 CET 2017


The macro qtu() is defined as follows:

    #define qtu( i ) ((i).toUtf8().constData())

"i" is a QString, .toUtf8() returns a QByteArray, .constData() returns a
pointer to the data inside the QByteArray.

This macro is used for the initialization of bookmark.psz_name, which
suffered from two problems.

Firstly, the pointer returned by .constData() is owned by the
QByteArray, so it must not be freed explicitly. This is not compatible
with how the bookmark is used (e.g. psz_name may be freed by
BooksmarksDialog::edit()).

Secondly, in the definition of qtu(), the QByteArray is a temporary
value. Therefore, it is "destroyed as the last step in evaluating the
full-expression that (lexically) contains the point where [it was]
created".

Concretely, this means that this call is correct:

    do_something( qtu( string ) );

But this one is undefined behavior:

    const char *s = qtu( string );
    do_something( s );

Thus, here, bookmark.psz_name was initialized with a pointer to garbage
data.

To fix both problems, just strdup() the string.

(Fixes invalid reads reported by valgrind)

Signed-off-by: Romain Vimont <rom at rom1v.com>
---
Note: without looking at the definition of qtu(), it is not obvious that the
result of qtu(str) has a shorter lifetime than str. Maybe its name should be
more explicit about this (qtu_tmp() or something).

 modules/gui/qt/dialogs/bookmarks.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/gui/qt/dialogs/bookmarks.cpp b/modules/gui/qt/dialogs/bookmarks.cpp
index c3c29a02fe..fd5fca147a 100644
--- a/modules/gui/qt/dialogs/bookmarks.cpp
+++ b/modules/gui/qt/dialogs/bookmarks.cpp
@@ -168,7 +168,7 @@ void BookmarksDialog::add()
     {
         QString name = THEMIM->getIM()->getName() + " #"
                      + QString::number( bookmarksList->topLevelItemCount() );
-        bookmark.psz_name = const_cast<char *>qtu( name );
+        bookmark.psz_name = strdup( qtu( name ) );
 
         input_Control( p_input, INPUT_ADD_BOOKMARK, &bookmark );
     }
-- 
2.11.0



More information about the vlc-devel mailing list