Date: Wed, 14 Sep 2005 07:13:37 -0700
From: "Ferguson, Justin (IARC)" <FergusonJ@...doe.gov>
To: "'Martin Roesch'" <roesch@...rcefire.com>
Cc: "'snort-devel@...ts.sourceforge.net'" <snort-devel@...ts.sourceforge.net>,
	"'snort-users@...ts.sourceforge.net'" <snort-users@...ts.sourceforge.net>,
	"'bugtraq@...urityfocus.com'" <bugtraq@...urityfocus.com>
Subject: FW: [Snort-devel] Re: Snort DoS Fallacies

This was returned due to size limits, I've removed the attached 2.4 snapshot
from the email and am resending-- original email below.


J. Ferguson
Intrusion Analyst
NNSA Information Assurance Response Center 
fergusonj@...doe.gov


-----Original Message-----
From: Ferguson, Justin (IARC) 
Sent: Wednesday, September 14, 2005 6:50 AM
To: 'Martin Roesch'; Ferguson, Justin (IARC)
Cc: 'snort-devel@...ts.sourceforge.net';
'snort-users@...ts.sourceforge.net'; 'bugtraq@...urityfocus.com'
Subject: RE: [Snort-devel] Re: [Snort-users] Snort DoS Fallacies


Martin, et al,

Most of what you've said in this last email I agree with and thus will just
edit out in my reply, but before I get to that something that was brought to
my attention last night that is somewhat concerning is that this bug was
first disclosed in Dec 2004 by a group called 'milw0rm', a quick google
search will bring up the exploit and quotes from irc of you talking about
the exact same bug. It would appear that somewhere this bug was remerged
into snort, which makes me wonder

1) what other bugs and such have made it back into the current versions? And
2) did this DoS affect sourcefire as well, or only the free version?

With that said, comments inline.


>Ok, I'll put aside all assumptions about your motives and just lay it
>out as straight as I'm able.

I appreciate the candor, and I assure you I have no political motivations in
my bringing this up. If that was my goal there are many other, more
effective ways to do this.. Such as posting the findings of a source audit I
was commissioned to do not long ago.

My one and only motivation in bringing this up was simply that people were
misinformed and my concern was that someone who may not be able to upgrade
for any number of reasons (outside the realm of snort itself), may say to
themselves 'well I don't use verbose/-v and therefore am fine' when in fact
because of any other circumstantial reasons they may in fact be vulnerable.
As a vendor, I, and many people see it as your responsibility to fully
disclose the details-- at least in regards to who is affected by a bug.

In fact, the only reason I found this was I was writing a patch for an
undisclosed 3rd party client who is one of those people who cannot upgrade
and wasn't affected by the vulnerability as it was disclosed, and I just
happened to grep out of habit after the patch was done.


>If you can't execute a code path that could result in an execution
>fault then it's not a bug.  For example, we don't validate that the  
>Packet pointer that gets passed into the decoder is valid because the  
>design of the program.  Anyway...

I agree with you, however if you are so sure of this, why even add the other
if ()'s?


>It becomes your job when you speak up, pundits add no value to open
>source projects and when you stepped up to "set the record straight"  
>with incorrect or incomplete data you don't get to get off the hook  
>by saying "it's not my job".

Agreed, and I will not say I did all of my homework on this particular part.
I did not check under what conditions data->packet_flag was set, and made
the apparently incorrect assumption that -A fast allowed the execution path.
I say apparently, because I still have no confirmed in the code that you are
correct, I'm just taking your word (and code snippets) on it.

>Case closed on that one, don't use the "packet" option if you're
>using "output alert_fast" in snort.conf and otherwise you're set at  
>the command line with "-A fast".

Agreed, and to be pedantic, this was only my point that it *could* happen,
not that many or even anyone was actually doing it-- just to mention it in
case someone was, I was just incorrect on the exact details.



>If you run with ASCII logging you can be DoS'd at any time (fairly
>trivially) and that code will not be fixed because it works as  
>designed.  This problem pales in comparison IMO.

Yes and no, some people have ungodly amounts of disk space (and thus lots of
inodes), and some people set their logs to eat themselves or rotate them out
(yes, I am aware of why you shouldn't do that, comments not needed)- in
which case this would be a more effective DoS. Additionally, if I can
generate enough traffic, I can DoS you by just causing enough alerts and
making you run out of disk space-- yes someone should be watching to prevent
this from ever happening, but its kind of the same deal.

Again, the point was not to argue the semantics of whether one should or
should not be running ASCII logging, but rather that if you are one of those
people doing that, you are vulnerable to this DoS even if you are not using
-v, despite what the advisor{y,ies) state.




>I pointed out that developers use the DEBUG statements because you
>didn't seem to be familiar with how that code is used and why it's  
>there.  If you really dig into it you'll see that not only do you  
>have to --enable-debug when you run ./configure but you also have to  
>set an environment variable (export SNORT_DEBUG=131072) even if debug  
>mode is enabled.

Working with the presumption that the code you were looking at didn't have
the #ifdef/if()PrintIPPkt() #endif PrintIPPkt() part, then yes I can see how
you might misunderstand my statement. I was just simply stating that in that
instance, with the code I was looking at, it made 0 sense to even have the
#ifdef's as the result was the same either way.


>Regardless, that's not the way the code is in 2.4.0 or CVS nor in any
>of the initial development versions from when I wrote it.  I don't  
>know where you got that code from but it looks like a bad CVS merge  
>on your end.  Download 2.4.0 or CVS on the SNORT_2_4 branch to see  
>the actual code (I did).

That's all fine and well, but I didn't actually check the code out of CVS, I
pulled down the snapshots-2.4 from snort.org/pub-bin/snapshots.cgi yesterday
and that's where the code existed, and I found it also in previous versions
of snort. I just pulled it down again and found it still exists in there/has
the same md5sum:

3050 #ifdef DEBUG_FRAG3
3051		/*
3052		 * Note, that this won't print out the IP Options or any
other
3053  	 * data this is established when the packet is decoded.
3054       	 */
3055		if (DEBUG_FRAG & GetDebugLevel()) 
3056		{
3057			ClearDumpBuf();
3058			printf("++++++++++++++++++Frag3 DEFRAG'd
PACKET++++++++++++++\n");
3059			PrintIPPkt(stdout, defrag_pkt->iph->ip_proto,
defrag_pkt);
3060			printf("++++++++++++++++++Frag3 DEFRAG'd
PACKET++++++++++++++\n");
3061			ClearDumpBuf();
3062		}
3063	#endif
3064			ClearDumpBuf();
3065			printf("++++++++++++++++++Frag3 DEFRAG'd
PACKET++++++++++++++\n");
3066			PrintIPPkt(stdout, defrag_pkt->iph->ip_proto,
defrag_pkt);
3067			printf("++++++++++++++++++Frag3 DEFRAG'd
PACKET++++++++++++++\n");
3068			ClearDumpBuf();

I think you should double check the code. As this DEBUG is pointless, and
this DoS is vuln in the frag3 preprocessor if its enabled.

$ md5sum snort-snapshots-2.4.tar.gz b96672bd923e130250a3099317572f66
snort-snapshots-2.4.tar.gz $ ls -alh snort-snapshots-2.4.tar.gz
-rw-r--r--    1 jferguson jferguson         1.9M Sep 14 13:03
snort-snapshots-2.4.tar.gz

The source is also attached.


>For giggles
>I rigged the POC to try every possible value of TCP option and length  
>with a variety of option sizes, it didn't cause any problems so I'd  
>say that the code is conclusively not vulnerable to this issue  
>outside of the SACK option in TCP option processing.

We both know that fuzzing/black boxing of this sort proves very little, if
you were that confident that the points could never be NULL then the other
if (...)'s wouldn't have been added, but it's a moot point and really not
worth the argument.

>I was merely pointing out the real actual issue as I saw it, you
>didn't make the point when in fact it was the most serious one.  

Which shows how easily this mistake that was made on your teams part can be
made. I grepped for PrintIPPkt() and not PrintTCPHeader() or whatever it
was.

>The
>DEBUG code is only called if you manually enable debug mode at  
>compile time and set an appropriate environment variable so that's a  
>complete non-issue as far as this is concerned.

In the stream4 preprocessor yes, in frag3, depending on which code you have,
having the debugging enabled just stops the DoS from happening twice, which
is an impossibility anyways.

>Saving face has nothing to do with it.  You made claims to be
>pointing out other issues that were incorrect  or required  
>expansion. 

The only things I've said that was incorrect was my assumption that -A fast
triggered the one in fast, and that arguably, that the other pointers could
possibly be NULL. Everything else was correct.

>The one real issue that people were likely to run into
>with the default configuration of a relatively commonly used option  
>was missed.  You also made several assumptions without testing them  
>and proposed to broaden the scope of the vulnerability to different  
>inputs without any proof or, ultimately, merit.

And your team, the vendor missed all of the above on your first *two* passes
through, once in Dec 2004, and once in Aug/Sep 2005, does this mean what
you've said and/or done is without merit as well? You guy's obviously didn't
do much testing either. Furthermore, you have useless #ifdef's that you
don't even know exist, is this supposed to inspire confidence in you or your
team and make us want to buy your product?



>1) The DoS effects "-A fast"  - this was wrong, it only effects the
>output directive with a little used optional argument

But it still affects those few people, which would not have known this by
going by whats on the snort news/advisories.

>2) ASCII logging also has the problem - right, but if you run with
>ASCII logging you're subject to other DoS's as well that are endemic  
>to the configuration

The other DoS's are a known issue, this was a non-known issue that in quite
a big quieter, and again was not referenced and still not referenced in your
bug report.


>3) Other TCP options can cause the same DoS  - this was wrong

I have no proof otherwise, so I won't argue.

>4) The DoS effects frag3 and stream4 - this was wrong, there's no
>practical configuration that  will result in a DoS with frag3/stream4

You are incorrect, as I showed above. Frag3 has the problem without
debugging. And either way, here is another possible avenue, regardless of
whether its plausible or not, its still possible.. You'd think as a security
vendor you would want people to know this, but it appears that you are more
concerned with saying 'these issues are a non-issue, you are only vuln if
you use -v' than correcting the mistake and saying, 'if in the unlikely
event you are using one of these non-recommended configurations, then you
are also subject to this problems, but you shouldn't be doing that anyways'.
 

>We payed attention to the initial data from the original reporter and
>said "yep, -v is a problem" and left it at that.

Did you not at least get dejavu from 2004?

>Once you pointed
>out that there are some other paths in there it spurred me to take a  
>look (we haven't really done much with that code in years, it really  
>should not be used for production as I've said) and the full analysis  
>is as I described in this email and the last.

Minus the mistake(s) I've pointed out in this email, namely the frag3 thing.

>The fixes in CVS fix
>all the problems just like the fix the original problem, so if you  
>absolutely must have a fix today then do as I said and grab log.c  
>from CVS and recompile.

You do not find it odd at all, that you say 'no one should be doing xyz on
production sensors anyways', and then you suggest that they run CVS code on
production sensors?

Which one do you think will result in problems first?

>It doesn't appear that there's anything more to say on this topic
>unless you have some more observations you'd like to make.

I'm attaching a patch that may/may not cleanly patch against all previous
versions of snort, it's been tested with 2.3.3. Regardless someone who
cannot upgrade to CVS and needs ASCII/frag3/-A full/-A fast + conf
options/to sleep well at night not wondering if there is another execution
path that we both missed can simply look at the patch and hand merge the
changes into log.c, they are fairly trivial and the patch is quite straight
forward.

Best Regards,

J. Ferguson
Intrusion Analyst
NNSA Information Assurance Response Center (IARC) fergusonj@...doe.gov




Download attachment "snort-2.3.3.diff" of type "application/octet-stream" (4659 bytes)

Powered by blists - more mailing lists