Merge remote-tracking branch 'origin/topic/johanna/bit-1463'

* origin/topic/johanna/bit-1463:
  Refactor oob tests using different approach.
  Add a number of out_of_bound checks to Packet.cc

BIT-1463 #merged
This commit is contained in:
Robin Sommer 2015-08-31 14:35:49 -07:00
commit 0494a6d882
4 changed files with 64 additions and 16 deletions

View file

@ -47,6 +47,12 @@ void Packet::Init(int arg_link_type, struct timeval *arg_ts, uint32 arg_caplen,
l2_valid = false; l2_valid = false;
if ( data && cap_len < hdr_size )
{
Weird("truncated_link_header");
return;
}
if ( data ) if ( data )
ProcessLayer2(); ProcessLayer2();
} }
@ -94,6 +100,7 @@ void Packet::ProcessLayer2()
bool have_mpls = false; bool have_mpls = false;
const u_char* pdata = data; const u_char* pdata = data;
const u_char* end_of_data = data + cap_len;
switch ( link_type ) { switch ( link_type ) {
case DLT_NULL: case DLT_NULL:
@ -140,6 +147,12 @@ void Packet::ProcessLayer2()
// 802.1q / 802.1ad // 802.1q / 802.1ad
case 0x8100: case 0x8100:
case 0x9100: case 0x9100:
if ( pdata + 4 >= end_of_data )
{
Weird("truncated_link_header");
return;
}
vlan = ((pdata[0] << 8) + pdata[1]) & 0xfff; vlan = ((pdata[0] << 8) + pdata[1]) & 0xfff;
protocol = ((pdata[2] << 8) + pdata[3]); protocol = ((pdata[2] << 8) + pdata[3]);
pdata += 4; // Skip the vlan header pdata += 4; // Skip the vlan header
@ -154,6 +167,12 @@ void Packet::ProcessLayer2()
// Check for double-tagged (802.1ad) // Check for double-tagged (802.1ad)
if ( protocol == 0x8100 || protocol == 0x9100 ) if ( protocol == 0x8100 || protocol == 0x9100 )
{ {
if ( pdata + 4 >= end_of_data )
{
Weird("truncated_link_header");
return;
}
inner_vlan = ((pdata[0] << 8) + pdata[1]) & 0xfff; inner_vlan = ((pdata[0] << 8) + pdata[1]) & 0xfff;
protocol = ((pdata[2] << 8) + pdata[3]); protocol = ((pdata[2] << 8) + pdata[3]);
pdata += 4; // Skip the vlan header pdata += 4; // Skip the vlan header
@ -164,6 +183,12 @@ void Packet::ProcessLayer2()
// PPPoE carried over the ethernet frame. // PPPoE carried over the ethernet frame.
case 0x8864: case 0x8864:
if ( pdata + 8 >= end_of_data )
{
Weird("truncated_link_header");
return;
}
protocol = (pdata[6] << 8) + pdata[7]; protocol = (pdata[6] << 8) + pdata[7];
pdata += 8; // Skip the PPPoE session and PPP header pdata += 8; // Skip the PPPoE session and PPP header
@ -230,6 +255,12 @@ void Packet::ProcessLayer2()
{ {
// Assume we're pointing at IP. Just figure out which version. // Assume we're pointing at IP. Just figure out which version.
pdata += GetLinkHeaderSize(link_type); pdata += GetLinkHeaderSize(link_type);
if ( pdata + sizeof(struct ip) >= end_of_data )
{
Weird("truncated_link_header");
return;
}
const struct ip* ip = (const struct ip *)pdata; const struct ip* ip = (const struct ip *)pdata;
if ( ip->ip_v == 4 ) if ( ip->ip_v == 4 )
@ -254,18 +285,18 @@ void Packet::ProcessLayer2()
while ( ! end_of_stack ) while ( ! end_of_stack )
{ {
end_of_stack = *(pdata + 2) & 0x01; if ( pdata + 4 >= end_of_data )
pdata += 4;
if ( pdata >= pdata + cap_len )
{ {
Weird("no_mpls_payload"); Weird("truncated_link_header");
return; return;
} }
end_of_stack = *(pdata + 2) & 0x01;
pdata += 4;
} }
// We assume that what remains is IP // We assume that what remains is IP
if ( pdata + sizeof(struct ip) >= data + cap_len ) if ( pdata + sizeof(struct ip) >= end_of_data )
{ {
Weird("no_ip_in_mpls_payload"); Weird("no_ip_in_mpls_payload");
return; return;
@ -288,13 +319,14 @@ void Packet::ProcessLayer2()
else if ( encap_hdr_size ) else if ( encap_hdr_size )
{ {
// Blanket encapsulation. We assume that what remains is IP. // Blanket encapsulation. We assume that what remains is IP.
pdata += encap_hdr_size; if ( pdata + encap_hdr_size + sizeof(struct ip) >= end_of_data )
if ( pdata + sizeof(struct ip) >= data + cap_len )
{ {
Weird("no_ip_left_after_encap"); Weird("no_ip_left_after_encap");
return; return;
} }
pdata += encap_hdr_size;
const struct ip* ip = (const struct ip *)pdata; const struct ip* ip = (const struct ip *)pdata;
if ( ip->ip_v == 4 ) if ( ip->ip_v == 4 )

View file

@ -3,38 +3,48 @@
#empty_field (empty) #empty_field (empty)
#unset_field - #unset_field -
#path weird #path weird
#open 2012-04-11-16-01-35 #open 2015-08-31-21-35-27
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer
#types time string addr port addr port string string bool string #types time string addr port addr port string string bool string
1334160095.895421 - - - - - truncated_IP - F bro 1334160095.895421 - - - - - truncated_IP - F bro
#close 2012-04-11-16-01-35 #close 2015-08-31-21-35-27
#separator \x09 #separator \x09
#set_separator , #set_separator ,
#empty_field (empty) #empty_field (empty)
#unset_field - #unset_field -
#path weird #path weird
#open 2012-04-11-14-57-21 #open 2015-08-31-21-35-27
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer
#types time string addr port addr port string string bool string #types time string addr port addr port string string bool string
1334156241.519125 - - - - - truncated_IP - F bro 1334156241.519125 - - - - - truncated_IP - F bro
#close 2012-04-11-14-57-21 #close 2015-08-31-21-35-27
#separator \x09 #separator \x09
#set_separator , #set_separator ,
#empty_field (empty) #empty_field (empty)
#unset_field - #unset_field -
#path weird #path weird
#open 2012-04-10-21-50-48 #open 2015-08-31-21-35-28
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer
#types time string addr port addr port string string bool string #types time string addr port addr port string string bool string
1334094648.590126 - - - - - truncated_IP - F bro 1334094648.590126 - - - - - truncated_IP - F bro
#close 2012-04-10-21-50-48 #close 2015-08-31-21-35-28
#separator \x09 #separator \x09
#set_separator , #set_separator ,
#empty_field (empty) #empty_field (empty)
#unset_field - #unset_field -
#path weird #path weird
#open 2012-05-29-22-02-34 #open 2015-08-31-21-35-30
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer
#types time string addr port addr port string string bool string #types time string addr port addr port string string bool string
1338328954.078361 - - - - - internally_truncated_header - F bro 1338328954.078361 - - - - - internally_truncated_header - F bro
#close 2012-05-29-22-02-34 #close 2015-08-31-21-35-30
#separator \x09
#set_separator ,
#empty_field (empty)
#unset_field -
#path weird
#open 2015-08-31-21-35-30
#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer
#types time string addr port addr port string string bool string
0.000000 - - - - - truncated_link_header - F bro
#close 2015-08-31-21-35-30

Binary file not shown.

View file

@ -19,4 +19,10 @@
# @TEST-EXEC: bro -r $TRACES/trunc/icmp-header-trunc.pcap # @TEST-EXEC: bro -r $TRACES/trunc/icmp-header-trunc.pcap
# @TEST-EXEC: cat weird.log >> output # @TEST-EXEC: cat weird.log >> output
# Truncated packets where the captured length is less than the length required
# for the packet header should also raise a Weird
# @TEST-EXEC: bro -r $TRACES/trunc/trunc-hdr.pcap
# @TEST-EXEC: cat weird.log >> output
# @TEST-EXEC: btest-diff output # @TEST-EXEC: btest-diff output