From 583150198b78c84d043455b0afcca58a9659eab3 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sun, 11 Sep 2016 01:16:24 +0200 Subject: extcap: fix use-after-free for preferences In commit v2.3.0rc0-117-g485bc45 (backported to v2.2.0rc0-44-g66721ca), extcap_prefs_dynamic_vals and extcap_cleanup were added in an attempt to address dangling pointers. Unfortunately it is not sufficient: - A pointer to the preference value is stored in extcap_arg and passed to the prefs API, but this extcap_arg structure can become invalid which result in use-after-free whenever the preference is accessed. - On exit, a use-after-free occurs in prefs_cleanup when the preference value is being checked. As the preference subsystem actually manages the memory for the string value and consumers should only provide a pointer where the value can be stored, convert the char* field in extcap to char**. This has as additional benefit that values are not limited to 256 bytes anymore. extcap_cleanup is moved after epan_cleanup to ensure that prefs_cleanup does not operate on dangling pointers. Crash is reproducible under ASAN with: tshark -i randpkt Ping-Bug: 12183 Change-Id: Ibf1ba1102a5633aa085dc278a12ffc05a4f4a34b Reviewed-on: https://code.wireshark.org/review/17631 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Roland Knall --- ui/qt/extcap_argument_file.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'ui/qt/extcap_argument_file.cpp') diff --git a/ui/qt/extcap_argument_file.cpp b/ui/qt/extcap_argument_file.cpp index df8da8caeb..5d7df3f5b5 100644 --- a/ui/qt/extcap_argument_file.cpp +++ b/ui/qt/extcap_argument_file.cpp @@ -55,7 +55,6 @@ ExtcapArgumentFileSelection::~ExtcapArgumentFileSelection() QWidget * ExtcapArgumentFileSelection::createEditor(QWidget * parent) { - QString storeval; QString text = defaultValue(); QString buttonText(UTF8_HORIZONTAL_ELLIPSIS); @@ -69,9 +68,10 @@ QWidget * ExtcapArgumentFileSelection::createEditor(QWidget * parent) textBox = new QLineEdit(text, parent); textBox->setReadOnly(true); - if ( _argument->storeval ) + const char *prefval = _argument->pref_valptr ? *_argument->pref_valptr : NULL; + if (prefval) { - QString storeValue = _argument->storeval; + QString storeValue(prefval); if ( storeValue.length() > 0 && storeValue.compare(text) != 0 ) text = storeValue.trimmed(); -- cgit v1.2.3