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 <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Roland Knall <rknall@gmail.com>

1 files changed, 5 insertions, 15 deletions

diff --git a/tfshark.c b/tfshark.c
index bc7323eee6..d2d7db5621 100644
--- a/tfshark.c
+++ b/tfshark.c
@@ -866,10 +866,8 @@ main(int argc, char *argv[])
        * cruft getting in the way. Makes the results of running
        * $ ./tools/valgrind-wireshark -n
        * much more useful. */
-#ifdef HAVE_EXTCAP
-      extcap_cleanup();
-#endif
       epan_cleanup();
+      extcap_cleanup();
       return 0;
     case 'O':        /* Only output these protocols */
       /* already processed; just ignore it now */
@@ -996,10 +994,8 @@ main(int argc, char *argv[])
     if (!dfilter_compile(rfilter, &rfcode, &err_msg)) {
       cmdarg_err("%s", err_msg);
       g_free(err_msg);
-#ifdef HAVE_EXTCAP
-      extcap_cleanup();
-#endif
       epan_cleanup();
+      extcap_cleanup();
       return 2;
     }
   }
@@ -1009,10 +1005,8 @@ main(int argc, char *argv[])
     if (!dfilter_compile(dfilter, &dfcode, &err_msg)) {
       cmdarg_err("%s", err_msg);
       g_free(err_msg);
-#ifdef HAVE_EXTCAP
-      extcap_cleanup();
-#endif
       epan_cleanup();
+      extcap_cleanup();
       return 2;
     }
   }
@@ -1057,10 +1051,8 @@ main(int argc, char *argv[])
     /* TODO: if tfshark is ever changed to give the user a choice of which
        open_routine reader to use, then the following needs to change. */
     if (cf_open(&cfile, cf_name, WTAP_TYPE_AUTO, FALSE, &err) != CF_OK) {
-#ifdef HAVE_EXTCAP
-      extcap_cleanup();
-#endif
       epan_cleanup();
+      extcap_cleanup();
       return 2;
     }
 
@@ -1098,10 +1090,8 @@ main(int argc, char *argv[])
   draw_tap_listeners(TRUE);
   funnel_dump_all_text_windows();
   epan_free(cfile.epan);
-#ifdef HAVE_EXTCAP
-  extcap_cleanup();
-#endif
   epan_cleanup();
+  extcap_cleanup();
 
   output_fields_free(output_fields);
   output_fields = NULL;