On Sun, Feb 04, 2007 at 02:53:36AM -0500, Axton wrote:
> >+ // only echo req/rep, timestamp, info req/rep have id/seq
> >+ if ($l4[0] == 0 || $l4[0] == 8 || ($l4[0] >= 13 && $l4[0] <=
> >16)) {
> >+ $data_header.= sprintf("%04s", dechex((float) $l4[3])); // id
> >+ $data_header.= sprintf("%04s", dechex((float) $l4[4])); //
> >seq
> >+ }
> It seems it would make more sense to not do this. This adds complexity to
> the code where it is not necessary. There are other type/code combinations
> that those this code consider, see
> http://www.erg.abdn.ac.uk/users/gorry/course/inet-pages/icmp-code.html.
> Even so, the packets logged by snort may not be of valid type/code
> combinations, in which case you would want to show the results. Kind of a
> garbage in, garbage out approach.
I may have missed a particular ICMP type that makes use of ICMP seq/id,
but if I understand what you are getting at, yes, it does make sense to
put these in the packet regardless of whether or not that ICMP type
used them.
Sadly, even though icmp_id and icmp_seq can be NULL according to the DB
schema, snort seems to put a 0 in there, so it doesn't appear to be
possible to detect whether there was actually no ICMP id/seq in the
original packet, or that they were 0.
That actually seems to be a bug in Snort, IMO.
-jon