2013-02-06

CurveDNS Bug

I get that it's an experimental functional implementation of a new security protocol, but I took a stab at building CurveDNS on OpenBSD today and found that there's a bug in its application of using the source IP option. It's part of the IPv4/IPv6 merged data structures that CurveDNS uses. ip_bind_random() is a wrapper around the bind() system call. I was scratching my head over this for awhile until it dawned on me that CurveDNS was using a union called "anysin_t":

typedef union {
        struct sockaddr sa;
        struct sockaddr_in sin;
        struct sockaddr_in6 sin6;
} anysin_t;

anysin_t is just an abstraction around sockaddr structs, but ip_bind_random() fails to use that abstraction and calls bind() directly:

if (bind(sock, (struct sockaddr *) &addr, addrlen) == 0)

The function includes this line twice, once in each branch of an if/else conditional that checks for the source IP address provided for either an AF_INET or AF_INET6 flag. I'd refactor this code were I writing it. Regardless, ip_bind_random() defines addr as an anysin_t:

anysin_t addr;

In order to get bind() to not fail with an "Invalid argument" error, you need to point out what type of sockaddr struct you are using in addr:

- if (bind(sock, (struct sockaddr *) &addr, addrlen) == 0)
+ if (bind(sock, (struct sockaddr *) &addr.sin6, sizeof(addr.sin6)) == 0)
return 1;

and also in the IPv4 section:

- if (bind(sock, (struct sockaddr *) &addr, addrlen) == 0)
+ if (bind(sock, (struct sockaddr *) &addr.sin, sizeof(addr.sin)) == 0)
return 1;

The proper fix for this is to use a better socket library or to write your own, but don't mix and match and ship a partial implementation.

No comments: