incorrect FDDI test in decode.c leads to reading uninitialized fields

[Tavis Ormandy <taviso () google com>]

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.

------------------------------------------------------------------------------
Master SQL Server Development, Administration, T-SQL, SSAS, SSIS, SSRS
and more. Get SQL Server skills now (including 2012) with LearnDevNow -
200+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only - learn more at:
http://p.sf.net/sfu/learnmore_122512
_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!