Snort mailing list archives
Re: incorrect FDDI test in decode.c leads to reading uninitialized fields
From: Joel Esler <jesler () sourcefire com>
Date: Tue, 15 Jan 2013 21:32:19 -0500
Tavis, Thanks for your report, we've opened a bug on our side to look into the issue. FDDI support isn't compiled into our commercial products and in the open source world you'd need to be on a FDDI datalink to have that decoder active and linked into the execution path of Snort. We look forward to any bugs that a man of your talents is able to provide to us, and we appreciate the review. Feel free to contact and coordinate with me directly or via bugs () snort org. Thanks -- Joel Esler Senior Research Engineer, VRT OpenSource Community Manager Sourcefire On Jan 15, 2013, at 4:05 PM, Tavis Ormandy <taviso () google com> 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.
------------------------------------------------------------------------------ Master Java SE, Java EE, Eclipse, Spring, Hibernate, JavaScript, jQuery and much more. Keep your Java skills current with LearnJavaNow - 200+ hours of step-by-step video tutorials by Java experts. SALE $49.99 this month only -- learn more at: http://p.sf.net/sfu/learnmore_122612
_______________________________________________ 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!
Current thread:
- incorrect FDDI test in decode.c leads to reading uninitialized fields Tavis Ormandy (Jan 15)
- Re: incorrect FDDI test in decode.c leads to reading uninitialized fields Victor Roemer (Jan 15)
- <Possible follow-ups>
- Re: incorrect FDDI test in decode.c leads to reading uninitialized fields Joel Esler (Jan 15)