On Sat, Jun 01, 2002 at 11:33:04PM +0900, Jun-ichiro itojun Hagino wrote:
> i guess it not robust to use TCHECK() and friends against values
> from the wire.
The current "TTEST2()" (which "TCHECK()" and friends use) should be safe
with the underflow check I added:
if the length is so large that "snapend - l" underflows (or if
it's treated as signed, and is negative), then "snapend - l"
will be greater than "snapend", and "TTEST2()" and friends will
fail, as they should;
if the length is not that large, but is large enough that
"snapend - l" is less than the address of the beginning of the
packet, it'll definitely be less than the object whose length is
being tested (as that object's address will be greater than the
address of the beginning of the packet), and "TTEST2()" and
friends will fail, as they should.
> also, we do not pass "bp + length" across functions,
> that prevents us from making proper boundary checks.
"Proper" as in checking both whether we run past the end of the captured
frame *and* whether we run past the length of, for example, the IP
datagram as specified in the IP header, so that we don't, for example,
dissect data in an Ethernet trailer as packet data?
Ethereal passes to its dissection routines a data structure that
contains a pointer to the beginning of the data to be dissected, and
both the captured and "reported" lengths of that data (caplen and len).
Each dissection routine, when it calls another dissection routine,
constructs a new data structure for the payload past the calling
dissection routine's headers, and hands that to the dissection routine
it's calling. The captured and reported lengths will both be reduced by
the length of the headers.
The IP dissection routine further adjusts the lengths to be no greater
than the length from the IP header, so that routines it calls can't go
past the end of the captured data *or* the length of the IP-datagram
part of the frame.
Currently, tcpdump doesn't distinguish between "ran past the end of the
packet" and "didn't run past the end of the packet but *did* run past
the end of the data we captured because the snapshot length was too
small to capture the entire packet". If we continue to do that, the
equivalent of what Ethereal does would be to have "ip_print()" do
if (bp + len < snapend)
snapend = bp + len;
after doing
len = ntohs(ip->ip_len);
if (length < len)
(void)printf("truncated-ip - %d bytes missing! ",
len - length);
len -= hlen;
len0 = len;
which would mean that "TTEST2()" and friends would, in routines called
from "ip_print()", fail if they run past the end of the IP datagram
length as reported in the IP header. (This includes the routines
checking IP options in the IP header, which is, I think, what we'd
want.)
-
This is the TCPDUMP workers list. It is archived at
http://www.tcpdump.org/lists/workers/index.html
To unsubscribe use mailto:[EMAIL PROTECTED]?body=unsubscribe
