[Snort-devel] Defect in spp_conversation.c

Denny Page denny at ...2179...
Sat Sep 13 20:07:39 EDT 2003


Following is a correction for preprocessors/spp_conversation.c.

While the problem is simple, the impact is incredibly high: multiple
conversations from between 2 hosts lost due to an error in the sort compare
routine.  Two things result from this.  First is excessive memory use in
conversation because we store the same conversation multiple times

The second problem is in spp_portscan2.    When spp_conversation looses the
conversation that it created based upon the original SYN, it creates another
conversation when the SYN-ACK is received.  Both packets are reported to
portscan2 as a new conversation.

This is most easily seen with a remote web server.  Consider the following
situation: We have spp_portscan2 enabled, and our local net is in the ignore
list.  A local client visits a remote web server, and requests a page that
has graphical navigation bars (lots of little images).  In the space of less
than a second, the local client makes 50 requests of the remote server.  The
SYN packet created conversations are ignored by configuration.  However,
since the SYN-ACK packets are viewed as a new conversations, spp_portscan2
erroneously reports that the local client is being scanned by the remote web
server (from port 80 no less :-).

Anyway, the following diff fixes the issue.  Cheers.

Denny


*** spp_conversation.c.org      2003-03-26 16:18:57.000000000 -0800
--- spp_conversation.c  2003-09-13 19:07:22.000000000 -0700
***************
*** 699,714 ****
          return -1;
      }

!     if(A->dport > B->dport) return -1;
!     if(A->dport < B->dport) return -1;

      /* now lets check the protocol, maybe this should be first but I
         think that most networks only see tcp traffic with a little
         DNS -- cmg
      */

!     if(A->ip_proto > B->ip_proto) return 1;
!     if(A->ip_proto < B->ip_proto) return -1;

  #ifdef DEBUG
      DebugMessage(DEBUG_CONVERSATION, "returning 0 for session
equalness\n");
--- 699,732 ----
          return -1;
      }

!     if(A->dport > B->dport)
!     {
!         DEBUG_WRAP(DebugMessage(DEBUG_CONVERSATION,"returning 1\n"););
!       return 1;
!     }
!
!     if(A->dport < B->dport)
!     {
!         DEBUG_WRAP(DebugMessage(DEBUG_CONVERSATION,"returning -1\n"););
!       return -1;
!     }

      /* now lets check the protocol, maybe this should be first but I
         think that most networks only see tcp traffic with a little
         DNS -- cmg
      */

!     if(A->ip_proto > B->ip_proto)
!     {
!         DEBUG_WRAP(DebugMessage(DEBUG_CONVERSATION,"returning 1\n"););
!       return 1;
!     }
!
!     if(A->ip_proto < B->ip_proto)
!     {
!         DEBUG_WRAP(DebugMessage(DEBUG_CONVERSATION,"returning -1\n"););
!       return -1;
!     }

  #ifdef DEBUG
      DebugMessage(DEBUG_CONVERSATION, "returning 0 for session
equalness\n");





More information about the Snort-devel mailing list