Commit graph

7 commits

Author SHA1 Message Date
Jon Siwek
2000e2a424 GH-977: Improve pcap error handling
Switches from pcap_next() to pcap_next_ex() to better handle all error
conditions.  This allows, for example, to have a non-zero exit code for
a Zeek process that fails to fully process all packets in a pcap file.
2020-06-08 18:11:58 -07:00
Daniel Thayer
27a7276009 Fix the ip-broken-header.bro test on macOS
There is no xzcat command on macOS by default.
2018-04-23 17:06:01 -05:00
Johanna Amann
924ed053c7 Fix OOB read in Sessions.cc
IP packets that have a header length that is greater than the total
length of the packet cause a integer overflow, which cause range-checks
to fail, which causes OOB reads.

Furthermore Bro does not currently check the version field of IP packets
that are read from tunnels. I added this check - otherwhise Bro reports
bogus IP information in its error messages, just converting the data
from the place where the IP information is supposed to be to IPs.

This behavior brings us closer to what other software (e.g. Wireshark)
displays in these cases.
2017-10-19 10:29:29 -07:00
Jon Siwek
fcd735cecd Fix core.truncation unit test on macOS.
The pcap file format has a global header and a header per packet.  The
global header of the pcap in question had a snaplen of 1, but with
packet headers indicating the full number of bytes saved within the
file.  It seems like the pcap file must of been artifically edited in
order for it to be this way.

When reporting the captured length of a packet, Apple's version of
libpcap now seems to report the full number of bytes saved within the
pcap's per-packet headers, but other versions seem to report the snaplen
from the global pcap header.  This caused the core.truncation test to
behave differently on macOS from other platforms.

I've manually hexedit'd the pcap so that the snaplen is still 1, but
contains just a single packet with a pcap header indicating a length of
8, which is less than the size of the link layer header and so should
still test the original code path that the unit test intended to
exercise.
2017-08-11 14:03:06 -05:00
Johanna Amann
fd6f9e470f Add a number of out_of_bound checks to Packet.cc
Mostly this verifies that we actually have the full headers that we are
trying to read in a packet.

Addresses BIT-1463
2015-08-31 13:09:18 -07:00
Jon Siwek
0aecca979e Remove unnecessary assert in ICMP analyzer (addresses #822).
The ICMP/ICMPv6 analyzers function correctly when full packets have
not been captured, but everything up to and including the ICMP header
is there (e.g. the functions that inspect ICMP error message context
correctly check the caplen to see if more info can be extracted).

The "Should have been caught earlier already." comment may have referred
to NetSessions::CheckHeaderTrunc, which works as intended to catch cases
where the ICMP header is not there in full, but then the assert was
still not correctly formulated for that...

Also changed the ICMP checksum calculation to not occur when the full
packet has not been captured, which seems consistent with what the UDP
analysis does.
2012-05-29 17:29:11 -05:00
Jon Siwek
51bad73e1e Fixes for IPv6 truncation and ICMP/ICMP6 analysis.
- Add more guards against trying to analyze captured packets with a
  truncated IPv6 static header or extension header chain.

- Add back in the ICMP payload tracking for ICMP "connections".

- Fix 'icmp_context' record construction.  Some field assignments
  were mismatched for ICMP and ICMP6.  Source and destination
  addresses were set incorrectly for context packets that don't
  contain a full IP header.  Some fields for ICMP6 weren't filled out.

- Changed ICMP Time Exceeded packets to raise the 'icmp_time_exceeded'
  event instead of 'icmp_error_message'.

- Add unit tests for truncation and the main types of ICMP/ICMP6
  that have specific events.

- Documentation clarifications.
2012-04-11 16:27:31 -05:00