Re: suspisious getname() patch

[Guy Harris <guy () alum mit edu>]

On Mar 11, 2004, at 8:53 AM, Gisle Vanem wrote:

> Should we maybe add an extra argument to getname()?
> Something like:
>
> const char *getname(const u_char *ap, int from_capture)
> {
>   ...
>   if(from_capture && !TTEST2(*ap, sizeof(addr))) {
>   return NULL;
>  }
>
> ...
>
> #define ipaddr_string(p) getname((const u_char *)(p), 1)

Or should we remove the checks from "getname()", and do them in its 
callers, and callers of "ipaddr_string()", when necessary?

If we treat that as a reason to audit the file in which the calls are 
made, that might find other places where we should be doing bounds 
checks but aren't.

If those routines just return NULL if the pointer is outside the packet 
data, then a too-short packet could cause a core dump, as "printf()" or 
whatever might just dereference the null pointer, so I think you either 
have to

	1) *always* store the result of "ipaddr_string()" (and "getname()" in 
those cases where it's directly used in a dissector) into a variable, 
and check the variable for null before using it

or

	2) do the checks in the dissector (which we might already be doing in 
some cases).

-
This is the TCPDUMP workers list. It is archived at
http://www.tcpdump.org/lists/workers/index.html
To unsubscribe use mailto:tcpdump-workers-request () tcpdump org?body=unsubscribe