Fix construction of ip6_ah (Authentication Header) record values.

Authentication Headers with a Payload Len field set to zero would cause
a crash due to invalid memory allocation because the previous code
assumed Payload Len would always be great enough to contain all
mandatory fields of the header.  This changes it so the length of
the header is explicitly checked before attempting to extract fields
located past the minimum length (8 bytes) of an Authentication Header.

Crashes due to this are only possible when handling script-layer events
ipv6_ext_headers, new_packet, esp_packet, or teredo_*.  Or also when
implementing one of the discarder_check_* family of functions.
Otherwise, Bro correctly parses past such a header.
This commit is contained in:
Jon Siwek 2012-09-18 16:52:12 -05:00
parent 6fbbf28290
commit 392b99b2fa
5 changed files with 25 additions and 7 deletions

View file

@ -1135,10 +1135,10 @@ type ip6_ah: record {
rsv: count;
## Security Parameter Index.
spi: count;
## Sequence number.
seq: count;
## Authentication data.
data: string;
## Sequence number, unset in the case that *len* field is zero.
seq: count &optional;
## Authentication data, unset in the case that *len* field is zero.
data: string &optional;
};
## Values extracted from an IPv6 ESP extension header.

View file

@ -148,9 +148,14 @@ RecordVal* IPv6_Hdr::BuildRecordVal(VectorVal* chain) const
rv->Assign(1, new Val(((ip6_ext*)data)->ip6e_len, TYPE_COUNT));
rv->Assign(2, new Val(ntohs(((uint16*)data)[1]), TYPE_COUNT));
rv->Assign(3, new Val(ntohl(((uint32*)data)[1]), TYPE_COUNT));
rv->Assign(4, new Val(ntohl(((uint32*)data)[2]), TYPE_COUNT));
uint16 off = 3 * sizeof(uint32);
rv->Assign(5, new StringVal(new BroString(data + off, Length() - off, 1)));
if ( Length() >= 12 )
{
// Sequence Number and ICV fields can only be extracted if
// Payload Len was non-zero for this header.
rv->Assign(4, new Val(ntohl(((uint32*)data)[2]), TYPE_COUNT));
uint16 off = 3 * sizeof(uint32);
rv->Assign(5, new StringVal(new BroString(data + off, Length() - off, 1)));
}
}
break;

View file

@ -0,0 +1,2 @@
[orig_h=2000:1300::1, orig_p=128/icmp, resp_h=2000:1300::2, resp_p=129/icmp]
[ip=<uninitialized>, ip6=[class=0, flow=0, len=166, nxt=51, hlim=255, src=2000:1300::1, dst=2000:1300::2, exts=[[id=51, hopopts=<uninitialized>, dstopts=<uninitialized>, routing=<uninitialized>, fragment=<uninitialized>, ah=[nxt=58, len=0, rsv=0, spi=0, seq=<uninitialized>, data=<uninitialized>], esp=<uninitialized>, mobility=<uninitialized>]]], tcp=<uninitialized>, udp=<uninitialized>, icmp=<uninitialized>]

Binary file not shown.

View file

@ -0,0 +1,11 @@
# @TEST-EXEC: bro -r $TRACES/ipv6_zero_len_ah.trace %INPUT >output
# @TEST-EXEC: btest-diff output
# Shouldn't crash, but we also won't have seq and data fields set of the ip6_ah
# record.
event ipv6_ext_headers(c: connection, p: pkt_hdr)
{
print c$id;
print p;
}