Wireshark-dev: Re: [Wireshark-dev] External processes in Snort dissector - code execution
From: Peter Wu <peter@xxxxxxxxxxxxx>
Date: Wed, 30 Aug 2017 01:36:23 +0100
On Tue, Aug 29, 2017 at 10:13:04AM +0200, Jakub Zawadzki wrote:
> Hi Peter,
> 
> W dniu 2017-08-28 18:50, Peter Wu napisał(a):
> > This can especially problematic for services like Cloudshark and
> > Webshark (by Jakub). The former is not yet affected since it does not
> > use 2.4 code (yet?) but the latter seems theoretically vulnerable as it
> > has a setconf API function (I was not able to get it to work though as
> > setconf changes are not visible in dumpconf).
> 
> dumpconf now support dumping value of snort.binary
> (https://code.wireshark.org/review/23268/),

What I meant was that changing a pref like (say) zebra.tcp.port via
setconf does not show up in the next dumpcinf command. Today it suddenly
works, maybe I was not looking correctly or mistyped the parameter name.

> and sharkd setconf requested is blocked from webshark API
> (https://bitbucket.org/jwzawadzki/webshark/commits/2687eec6b0413462e072a660af96896ee7cd6c33).

I was able to set snort.binary and snort.config anyway (but it seems
that your sharkd is built from an older source without these fixes) so
it did not show up. For snort.alerts_source it takes some special
treatment (use name instead of description), but then it can be changed
too.

The final roadblock which prevented successful exploitation is that the
only entrypoint to open a new file (and trigger the init routine
"snort_start") is throug htle "load" command.

If you were wondering how to bypass the setconf filter:

 1. Use "req=se%09conf" in the URL
 2. Which gets interpreted as "se<TAB>conf" in Python
 3. Which is JSON-encoded as "se\tconf" (see ESCAPE_DCT in
    /usr/lib/python*/json/encoder.py for the mappings)
 4. The stub json_unescape_str in sharkd_session.c strips all
    backslashes, so the original command is interpreted as "setconf"

...which is another lesson that canonicalization should be done well.
Maybe you should just check the command for alpha(numeric) characters
and reject all others?


By the way, the original issue is still open, Snort can execute
arbitrary commands when configured. I'm not entirely sure how to handle
this nicely... IMO a dissector should not execute external processes.
Would it be feasible to split this functionality into a C or Lua plugin?
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl