diff options
author | Peter Wu <peter@lekensteyn.nl> | 2016-09-11 01:16:24 +0200 |
---|---|---|
committer | Roland Knall <rknall@gmail.com> | 2016-09-11 08:33:42 +0000 |
commit | 583150198b78c84d043455b0afcca58a9659eab3 (patch) | |
tree | e3ec231548eaf1b8a2de10ff75bf218d7f17169b /rawshark.c | |
parent | b82695d9976ebed00f34bfc45f0358db095e0670 (diff) |
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>
Diffstat (limited to 'rawshark.c')
-rw-r--r-- | rawshark.c | 16 |
1 files changed, 4 insertions, 12 deletions
diff --git a/rawshark.c b/rawshark.c index fe68b76f92..510ed80bcf 100644 --- a/rawshark.c +++ b/rawshark.c @@ -802,10 +802,8 @@ main(int argc, char *argv[]) cmdarg_err("%s", err_msg); g_free(err_msg); epan_free(cfile.epan); -#ifdef HAVE_EXTCAP - extcap_cleanup(); -#endif epan_cleanup(); + extcap_cleanup(); exit(2); } n_rfcodes++; @@ -826,10 +824,8 @@ main(int argc, char *argv[]) if (raw_cf_open(&cfile, pipe_name) != CF_OK) { epan_free(cfile.epan); -#ifdef HAVE_EXTCAP - extcap_cleanup(); -#endif epan_cleanup(); + extcap_cleanup(); exit(2); } @@ -850,10 +846,8 @@ main(int argc, char *argv[]) /* Process the packets in the file */ if (!load_cap_file(&cfile)) { epan_free(cfile.epan); -#ifdef HAVE_EXTCAP - extcap_cleanup(); -#endif epan_cleanup(); + extcap_cleanup(); exit(2); } } else { @@ -863,10 +857,8 @@ main(int argc, char *argv[]) } epan_free(cfile.epan); -#ifdef HAVE_EXTCAP - extcap_cleanup(); -#endif epan_cleanup(); + extcap_cleanup(); return 0; } |