[Snort-devel] incorrect FDDI test in decode.c leads to reading uninitialized fields

Victor Roemer vroemer at ...402...
Tue Jan 15 16:38:08 EST 2013


Hello Tavis, thanks for the bug report, I'll get this tracked on our side.


Victor

On Tue, Jan 15, 2013 at 4:22 PM, Tavis Ormandy <taviso at ...3361...> wrote:

> Hey, I've been looking at snort code recently, and have a few issues.
> I'll send more soon, but here is the first: DecodeFDDIPkt() in
> decode.c looks broken, you can see the intention of the author was to
> reject any packet that doesn't have FDDI_DSAP_IP and FDDI_SSAP_IP, but
> he incorrectly uses && instead of || in the condition at around line
> 6588.
>
> 6583     /*
> 6584      * Now let's see if we actually care about the packet... If we
> don't,
> 6585      * throw it out!!!
> 6586      */
> 6587     if((p->fddisaps->dsap != FDDI_DSAP_IP) &&
> 6588             (p->fddisaps->ssap != FDDI_SSAP_IP))
> 6589     {
> 6590         DEBUG_WRAP(
> 6591                 DebugMessage(DEBUG_DECODE,
> 6592                     "This FDDI Packet isn't an IP/ARP packet...\n");
> 6593                 );
> 6594         PREPROC_PROFILE_END(decodePerfStats);
> 6595         return;
> 6596     }
>
> This is a bug, because p->fddiiparp is only set when both fields are
> set, but that condition allows dsap or ssap to be set and still
> continue with it unintialized. The fix is trivial, just replacing '&&'
> with '||' should solve the problem.
>
> It's trivial to produce a minimal testcase, here's an example:
>
> $ xxd DecodeFDDIPkt.pcap
> 0000000: d4c3 b2a1 0200 0400 0000 0000 0000 0000  ................
> 0000010: 6000 0000 0a00 0000 0a4f de50 757f 0700  `........O.Pu...
> 0000020: 1000 0000 1000 0000 00aa aaaa aaaa aabb  ................
> 0000030: bbbb bbbb bb00 aa00                      ........
>
> (use xxd -r to recover)
>
> $ ./snort --pcap-single=- < DecodeFDDIPkt.pcap
> Running in packet dump mode
>
>         --== Initializing Snort ==--
> Initializing Output Plugins!
> pcap DAQ configured to read-file.
> The DAQ version does not support reload.
> Acquiring network traffic from "stdin".
>
>         --== Initialization Complete ==--
>
>    ,,_     -*> Snort! <*-
>   o"  )~   Version 2.9.4 GRE (Build 40)
>    ''''    By Martin Roesch & The Snort Team:
> http://www.snort.org/snort/snort-team
>            Copyright (C) 1998-2012 Sourcefire, Inc., et al.
>            Using libpcap version 1.1.1
>            Using PCRE version: 8.12 2011-01-15
>            Using ZLIB version: 1.2.3.4
>
> Commencing packet processing (pid=4154)
> Segmentation fault (core dumped)
>
> src/decode.h:#define FDDI_DSAP_IP                    0xaa    /* IP */
> src/decode.h:#define FDDI_SSAP_IP                    0xaa    /* IP */
>
> $ gdb -q ./snort
> (gdb) r -q --pcap-single=- < DecodeFDDIPkt.pcap
> Starting program: snort -q --pcap-single=- < DecodeFDDIPkt.pcap
>
> Program received signal SIGSEGV, Segmentation fault.
> DecodeFDDIPkt (p=0x7fffffffd320, pkthdr=0x7fffffffdef0, pkt=0x15ce0f0
> "") at decode.c:6600
> 6600        switch(htons(p->fddiiparp->ethertype))
> (gdb) p/x p->fddisaps->dsap
> $1 = 0x0
> (gdb) p/x p->fddisaps->ssap
> $2 = 0xaa
> (gdb)
>
> You can see that flow incorrectly continued past the ssap/dsap check.
>
> Hope this helps, Tavis.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20130115/25bf927e/attachment.html>


More information about the Snort-devel mailing list