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.
Ack, InternalError() is not something that external input should be
able to trigger. I already removed a number of these over time, but
never looked systematically for them.
I suspect that in most cases, a weird should be generated instead, and
Bro should just move on to the next packet.
Agreed, though sometimes they aren't about the traffic but about a
logic error in decoding it; it would be good to still differentiate
those cases from a broken packet, however indeed without aborting.
> I suspect that in most cases, a weird should be generated instead, and
> Bro should just move on to the next packet.
Agreed, though sometimes they aren't about the traffic but about a
logic error in decoding it; it would be good to still differentiate
those cases from a broken packet, however indeed without aborting.
In line with what you frame, the history behind these is that they're meant
to reflect should-never-happen situations; not "weird" activity, but apparent
internal inconsistencies in Bro's processing/execution. So they don't really
fit with the notion of "weird". (Of course it's definitely possible there's
some mission-creep and InternalError is misused when Weird really is the
right notion.)
That said, for sure I agree that we don't want to give adversaries a way
to tickle a Bro crash. So ideally the solution here would be to come up
with something similar to the weird/notice framework, but that expicitly
captures the notion that Bro-is-confused rather than
something-happened-on-the-network.