Subject: lib/19060: link_addr is very lax
To: None <gnats-bugs@gnats.netbsd.org>
From: None <dyoung@noam.onthejob.net>
List: netbsd-bugs
Date: 11/15/2002 00:26:49
>Number:         19060
>Category:       lib
>Synopsis:       link_addr considers too many strings to be link-level addresses
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Nov 14 22:27:00 PST 2002
>Closed-Date:
>Last-Modified:
>Originator:     dyoung@ojctech.com
>Release:        NetBSD 1.6I
>Organization:
	OJC Technologies
>Environment:
System: NetBSD noam.onthejob.net 1.6I NetBSD 1.6I (GENERIC) #23: Tue Oct 22 01:33:13 CDT 2002 root@noam.onthejob.net:/home/dyoung/anoncvs/src/sys/arch/i386/compile/GENERIC i386
Architecture: i386
Machine: i386
>Description:
        link_addr(3) accepts many strings that do not meet the spec for a
        link-level address given in "man 3 link_addr".

        E.g., all these are accepted

            wi0.a.b.c.d.e.f (first dot should be a colon)
            wi0.aw.b.c.d.e.f (aw does not belong in a valid link-level
                              address)
            wi0:10x20x30x40x50x60    (you get the idea)

>How-To-Repeat:
        This program, which parses its first argument using link_addr,
        accepts strings that are not valid link-level addresses as
        defined in the man page for link_addr(3).

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <net/if.h>
#include <net/if_dl.h>

int
main(int argc, char **argv)
{
        struct sockaddr_dl sdl;
        if (argc < 2) {
                fprintf(stderr, "Usage: %s lladdr\n", argv[0]);
                exit(1);
        }
        sdl.sdl_len = sizeof(sdl);
        link_addr(argv[1], &sdl);
        if (sdl.sdl_len == 0) {
                fprintf(stderr, "%s: badly formed link-level address\n",
                        argv[1]);
                fprintf(stderr, "%*.*s\n", sdl.sdl_alen + 1, sdl.sdl_alen + 1,
                        "^");
                exit(1);
        }
        printf("%s\n", link_ntoa(&sdl));
        return 0;
}

        E.g., it accepts

            wi0.a.b.c.d.e.f (first dot should be a colon)
            wi0.aw.b.c.d.e.f (aw does not belong in a valid link-level
                              address)
            wi0:10x20x30x40x50x60    (you get the idea)

        Each is interpreted as

            wi0:a.b.c.d.e.f,
            wi0:a.0.b.c.d.e.f, and
            wi0:10.20.30.40.50.60,

        respectively.

>Fix:

        Replace the existing link_addr with this.

	Possibly the format for a interface name should be relaxed.

	I have not produced a patch for the man page, yet.

/* On a syntax error, set sdl->xdl_len to 0. Set sdl->sdl_alen to
 * the number of characters read before the error.
 */
void
link_addr(addr, sdl)
        register const char *addr;
        register struct sockaddr_dl *sdl;
{
        register char *octetp, *octetplim;
        register int len;
        register const char *start;
        const char *nextaddr;

        start = addr;
        octetplim = sdl->sdl_len + (char *)(void *)sdl;
        octetp = sdl->sdl_data;

        (void)memset(&sdl->sdl_family, 0, (size_t)sdl->sdl_len - 1);
        sdl->sdl_family = AF_LINK;

        /* First, try to read a device name and unit, and a
         * colon---i.e., match "[a-zA-Z][a-zA-Z]+[0-9]+:".  If no
         * device is named, skip to reading an address.
         */
        while (*addr != 0 && isalpha(*addr)) addr++;

        if (*addr != 0 && addr != start) {
                const char *start2 = addr;
                while (*addr != 0 && isdigit(*addr)) addr++;
                if (start2 != addr && *addr == ':') {
                        sdl->sdl_nlen = addr - start;
                        (void)memcpy(sdl->sdl_data, start, sdl->sdl_nlen);
                        octetp = &sdl->sdl_data[sdl->sdl_nlen];
                        addr++;
                } else
                        addr = start;
        } else
                addr = start;

        /* Read an address---i.e., match "([0-9]{1,2}\.)*[0-9]{1,2}". */
        do {
                if (!isxdigit(*addr))
                        goto fail;

                *octetp++ = (char)strtol(addr, &nextaddr, 16);
                if (((nextaddr - addr) == 0) || ((nextaddr - addr) > 2))
                        goto fail;
                addr = nextaddr;

                if (addr[0] == '.' && addr[1] == '0') {
                        addr++;
                        goto fail;
                } else if (addr[0] == '.') {
                        addr++;
                } else if (*addr == 0) {
                        break;
                }
        } while (octetp < octetplim);

        sdl->sdl_alen = octetp - LLADDR(sdl);
        len = octetp - (char *)(void *)sdl;
        if ((size_t) len > sizeof(*sdl))
                sdl->sdl_len = len;
        return;
fail:
        if (addr - start > sdl->sdl_alen) {
                sdl->sdl_alen = addr - start;
        }
        sdl->sdl_len = 0;
        return;
}
>Release-Note:
>Audit-Trail:
>Unformatted: