[Bro-Dev] [JIRA] (BIT-1045) Review usage of InternalError when parsing network traffic

[Robin Sommer (JIRA)]

    [ https://bro-tracker.atlassian.net/browse/BIT-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14213#comment-14213 ] 

Robin Sommer commented on BIT-1045:
-----------------------------------

Ok, good job on this. I went through and put a few small tweaks into topic/robin/internal-errors-merge. If you could double-check I'd appreciate (and it remains all fuzzy, I actually changed my mind a few times back and forth in some cases …)

Also, one remaining (or new) concern is that now methods which previously aborted, may now return a value that's not usable upstream (null pointers in particular). I saw that you adapted a few locations, but it's hard to tell if there are more. I'm wondering though if Coverity might help us here and flag, e.g., potential null pointer usages. 

> Review usage of InternalError when parsing network traffic
> ----------------------------------------------------------
>
>                 Key: BIT-1045
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-1045
>             Project: Bro Issue Tracker
>          Issue Type: Task
>          Components: Bro
>    Affects Versions: git/master, 2.1
>            Reporter: Vlad Grigorescu
>            Assignee: Robin Sommer
>
> Creating issue for tracking purposes.
> Reporter->InternalError denotes a fatal error, and will cause Bro to stop. Calling this function when parsing network traffic creates the possibility for an attacker using a "packet of death," which could stop Bro.
> I suspect that in most cases, a weird should be generated instead, and Bro should just move on to the next packet. A quick grep shows some likely candidates for incorrect use of InternalError:
> src/Sessions.cc:		reporter->InternalError("Bad IP protocol version in DoNextInnerPacket");
> src/Sessions.cc:		reporter->InternalError("fragment block not in dictionary");
> src/Sessions.cc:		reporter->InternalError("fragment block missing");
> src/Sessions.cc:			reporter->InternalError("unknown transport protocol");
> src/Frag.cc:		reporter->InternalError("bad IP version in fragment reassembly");
> src/IP.cc:		reporter->InternalError("IPv6_HdrChain::Init with truncated IP header");
> src/IP.cc:			reporter->InternalError("IPv6_Hdr_Chain bad header %d", type);
> src/IP.h:			reporter->InternalError("bad IP version in IP_Hdr ctor");
> src/RSH.cc:		reporter->InternalError("multiple rsh client names");
> src/RSH.cc:		reporter->InternalError("multiple rsh initial client names");
> src/POP3.cc:		reporter->InternalError("command not known");
> src/Rlogin.cc:		reporter->InternalError("multiple rlogin client names");
> src/ICMP.cc:			reporter->InternalError("unexpected IP proto in ICMP analyzer: %d",
> src/ICMP.cc:		reporter->InternalError("unexpected next protocol in ICMP::DeliverPacket()");
> src/SMB.cc:		reporter->InternalError("command mismatch for ParseTransaction");
> src/HTTP.cc:		reporter->InternalError("unrecognized HTTP message event");
> src/HTTP.cc:		reporter->InternalError("HTTP ParseRequest failed");
> src/DPM.cc:		reporter->InternalError("unknown protocol");
> src/RPC.cc:		reporter->InternalError("RPC underflow");
> src/RPC.cc:			reporter->InternalError("RPC resync: skipping over data failed");
> src/RPC.cc:					reporter->InternalError("inconsistent RPC record marker extraction");



--
This message was sent by Atlassian JIRA
(v6.1-OD-09-WN#6144)