We've already done checks to see whether we'll run past the end of the
packet, so there's no need to see whether fn_printn() did so.
Squelches some Coverity complaints.
The payload of a Security Association has a sequence of proposal
substructures; the Last Substruc field should only be 0 (for the last
proposal substructure) or 2 (if there's another proposal substructure
after the current one). If it's neither, don't try to dissect the next
item as a payload with the Last Substruc field's value as a payload
type.
The payload of a proposal substructure has a sequence of transform
substructures; the Last Substruc field should only be 0 (for the last
transform substructure) or 3 (if there's another transform substructure
after the current one). If it's neither, don't try to dissect the next
item as a payload with the Last Substruc field's value as a payload
type.
That keeps us from trying to, for example, dissect a bogus substructure
as an encrypted payload item and passing a null pointer as the struct
isakmp structure pointer.
Do more checks while we're at it.
Change cddcb5632d changed isonsap_string()
to take, as arguments, a pointer to the first octet of an NSAP and the
length of the NSAP, rather than a pointer to a string of octets the
first octet of which is the NSAP length and the subsequent octets are
the octets of the NSAP.
However, lookup_nsap() was not changed in a similar fashion, and
isonsap_string() handed it a pointer to the first octet of the NSAP,
which lookup_nsap() treated as the NSAP length.
This should fix GitHub issue #563.
Those are needed in addition to the checks against li.
This should fix GitHub issue #562. I suspect issue #563 is a separate
problem.
Tweak length check messages to be more like the IS-IS ones, and fix both
to print unsigned values with %u, while we're at it.
The function sig_print() did receive a correct caplen parameter value
but didn't use it correctly and could overread by one byte as Brian
Carpenter has demonstrated. Fix it by switching to the standard macros.
Check that the destination and source addresses are present before
printing them.
Check the length value from the length indiator as we dissect the CLNS
header. Make sure that header doesn't go past the on-the-network length
of the packet.
Check to make sure an option's content doesn't go past the length of the
option.
Also, don't print the body of an unknown option type twice with -vv and
more.
Kamil Frankowicz had found that truncated BE_STR and BE_SEQ ASN.1
elements could lead to an overread, from the source code it looked like
other ids could have this problem too. Move the checks introduced in
commit 72e501f out of the switch blocks to cover all ids by default.
This fixes GH#559 and GH#566.
Brian Carpenter had found that regardless of CVE-2016-8575 q933_print()
still could overread the buffer trying to parse a short packet. This
change fixes the problem.
In ethertype_print(), isoclns_print() and snap_print() adjust the length
arithmetics along the same lines as for ether_print() in the previous
commit. Where done, the current pointer is not greater than snapend so
that the difference (i.e. caplen) is never negative.
This does not fix a reported issue but the problem was very likely to be
there.
In that function the "length" parameter means off-the-wire length, that
is, the length declared inside the outer header. The "caplen" parameter
means the amount of bytes actually available in the captured packet.
gre_print_0() and the functions modelled after it passed the value of
"length" instead of the value of "caplen", this could make ether_print()
access beyond the memory allocated for the captured packet. Brian
Carpenter had demonstrated this for the OTV case.
Fix the involved functions that call ether_print() to pass the correct
value and leave a comment to dismiss "caplen" later as its value can be
reliably derived from the other ether_print() parameters.
Interleave the bounds checking with printing to make it visible which
last protocol field was OK. This fixes a vulnerability discovered by
Brian Carpenter.
Add a few checks to ip6_print() to make it stop decoding the IPv6
headers immediately when the header-specific functions signal an error
condition. Without this it tried to fetch the next header selector for
the next round regardless and could run outside of the allocated packet
space on a specially crafted IPv6 packet.
Brian Carpenter has demonstrated this for the Hop-by-Hop Options header.
Fix that specific case and also the Destination Options and Fragment
header processing as those use the same logic.
As it was correctly pointed out in GitHub issue #516, the TCPOPT_TCPAO
(formerly TCPOPT_AUTH) case had an issue with option length processing,
though without significant consequences thanks to a check elsewhere.
Besides that, the old code (introduced in 2005) decoded a structure
similar to a proposed encoding variant of the early (first published in
2007) revisions of the Internet-Draft but different from the encoding
of RFC 5925 (published in 2010). These issues are now addressed and the
TCP option renamed to TCP-AO.
Edit the .pcap file to change the TCP option kind from 20 to 29 to
match the changes done to the decoder. Now the code flow and hence
the text output are back to how they were before that change.
Have asn1_print() print out OIDs regardless of whether we have any
modules loaded or not.
Have smi_decode_oid decode the OID to an array of unsigned ints
regardless of whether we have any modules loaded or not.
Have smi_print_variable() just use asn1_print() to print the OID of a
variable binding if we don't have any modules loaded; in that case,
we're not going to try to look the OID up with libsmi, so we don't need
a decoded version.
Have smi_print_value() not bother decoding the OID or looking the OID up
if we don't have any modules loaded; also, if we *do* have modules
loaded, check whether smi_decode_oid() succeeds.
Don't call strtol() on the contents of a packet; there is *no* guarantee
that it won't run past the end of the buffer, as the buffer isn't a
null-terminated string. Instead, have our own routine to parse ASCII
numbers (based on the FreeBSD strtol()), which uses ND_TCHECK() and
checks against the on-the-wire length to ensure it doesn't go past the
end of the packet or the end of the captured data. Have it check for
other errors as well, such as checking for negative lengths that aren't
-1.
Clean up other aspects of the packet parsing. Have them check the
on-the-wire length as well as the captured length.
Update the results of the resp_3 test.
Have the OID prefixes be arrays of uint8_t, and put the size of the
array into the list, rather than having them be "strings" and et the
length with strlen().
Have a macro to encapsulate X.690 section 8.19.4's rules for the first
octet of an OID value, and use it; that makes the components of the OID
clearer.
Also, if the prefix is longer than the remaining data in the OID - or
the remaining captured data - just skip it, don't treat that as an
error.
Catch INTEGER values with a length of 0, so we don't fetch a byte that
doesn't belong to the value.
Fix what appears to be a long-standing bug in the OID prefix matching
code, wherein the length of the *first* prefix in the table is used as
the length of *all* prefixes, and add some packet-length checking to
that list.
Report packets with an invalid SNMP version number as being SNMP, so
we at least indicate *that*.
At the beginning, make sure the on-the-wire length is >= the size of the
EGP header, and make sure the captured data is >= the size of the EGP
header, rather than making sure we have all the captured data, as we're
only looking at the header there.
Do on-the-wire length checking in egpnrprint().
If the L of the V of the TLV isn't large enough for everything it's
supposed to contain, just quit processing the TLV, print its contents in
hex, and process the next TLV.
Add multiple bounds and length checks to make sure we don't run past the
end of the packet.
Don't check the length of "end of TLV" indicators; 802.3 says it should
be ignored.
The Event Notification OAMPDU has a sequence number; report it.
print-lspping.c was written to one of the draft-ietf-mpls-lsp-ping-13
drafts; incorporate subsequent changes that are in RFC 4379. Not all
LV and subTLV types from that RFC are currently dissected.
Apparently, the IANA has two separate but similar registries, the BGP
Layer 2 Encapsulation Types registry and the MPLS Pseudowire Types
registry. Have two separate tables for them, and use the tables as
appropriate. Update them to match the current state of the registries.
11 is not the subTLV code for "BGP labeled IPv4 prefix" (and never was,
from what I can tell from looking at the I-Ds), 12 is.
Do more bounds checking.
Pull the code in asn1_print() to print octet sequences and (presumed)
printable strings into routines of their own, and use them when we're
printing them outside asn1_print().
That fixes some cases where we can run past the end of the packet
buffer.
NFSv2 file handles are always 32 bytes long, possibly with zero padding
at the end.
NFSv3 file handles are variable-length, however, so we cannot assume
that they have any minimum number of bytes of data; check that bytes
are present before looking at them.
