[quagga-dev 3077] Re: bugs related to length of interface names

Andrew J. Schorr aschorr at telemetry-investments.com
Fri Apr 1 22:42:47 BST 2005


On Fri, Apr 01, 2005 at 12:58:12PM -0500, Andrew J. Schorr wrote:
> For example, look in zebra/if_ioctl.c:interface_list_ioctl().
> It includes a line that looks like this:
> 
>    ifp = if_get_by_name (ifreq->ifr_name);
> 
> Is that valid?  Or is it a bug?  I don't know.  Is ifreq->ifr_name
> guaranteed to have a NUL termination char?  If it is, then the code
> is probably OK, except for the risk of false matches if the length
> somehow exceeds INTERFACE_NAMSIZ.  But if it is a fixed-size buffer
> of length IFNAMSIZ, then this call could run off the end of the buffer.

Or to take another example, look in
zebra/if_ioctl_solaris.c:interface_list_ioctl().  It says:

   ifp = if_get_by_name (lifreq->lifr_name);

Looking at the definition of struct lifreq, it says:

#define LIFNAMSIZ       32

struct  lifreq {
        char    lifr_name[LIFNAMSIZ];           /* if name, e.g. "en0" */
	...
}

Interestingly, LIFNAMSIZ is larger than quagga INTERFACE_NAMSIZ (20 bytes).
I'm not sure whether that means that Solaris can have interface names up
to 32 bytes.  And I'm not sure whether lifr_name is guaranteed to have
a '\0' termination char.  But in any case, the safer call would look
like this:

   ifp = if_get_by_name_len(lifreq->lifr_name,
			    strnlen(lifreq->lifr_name,
				    sizeof(lifreq->lifr_name)));

Then we would be 100% safe.  Certainly the existing code which assumes
that lifreq->lifr_name has a maximum length of INTERFACE_NAMSIZ bytes
does not appear to be correct...

Regards,
Andy



More information about the Quagga-dev mailing list