From cfcb9626d95640e42972c2284c08b61240f11d0c Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Wed, 21 Jul 2010 11:15:21 -0400 Subject: inet: null terminate ifr_name buffer The buffer holding the interface name should be null terminated. See man 7 socket on Linux or the definition of IFNAMSIZ in net/if.h on FreeBSD: Length of interface external name, including terminating '\0'. --- erts/emulator/drivers/common/inet_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 0ea54930ba..9ad6e4a845 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -4103,7 +4103,7 @@ static int inet_ctl_ifget(inet_descriptor* desc, char* buf, int len, goto error; sys_memset(ifreq.ifr_name, '\0', IFNAMSIZ); sys_memcpy(ifreq.ifr_name, buf+1, - (namlen > IFNAMSIZ) ? IFNAMSIZ : namlen); + (namlen >= IFNAMSIZ) ? IFNAMSIZ-1 : namlen); buf += (namlen+1); len -= (namlen+1); sptr = sbuf; @@ -4256,7 +4256,7 @@ static int inet_ctl_ifset(inet_descriptor* desc, char* buf, int len, goto error; sys_memset(ifreq.ifr_name, '\0', IFNAMSIZ); sys_memcpy(ifreq.ifr_name, buf+1, - (namlen > IFNAMSIZ) ? IFNAMSIZ : namlen); + (namlen >= IFNAMSIZ) ? IFNAMSIZ-1 : namlen); buf += (namlen+1); len -= (namlen+1); -- cgit v1.2.3 From 227d3f3ff078426ae79b7f2313bc8be915c5e252 Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Fri, 23 Jul 2010 11:28:46 -0400 Subject: inet: fix ifr_name buffer overflow The byte holding the length of the interface name for the ifget/2 functions is used in a signed context and can become negative, causing the ifreq.ifr_name buffer to be overrun. Test case: inet:ifget(lists:duplicate(128, "x"), [addr]). --- erts/emulator/drivers/common/inet_drv.c | 6 +++--- lib/kernel/test/inet_SUITE.erl | 12 ++++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 9ad6e4a845..87dc63509d 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -3905,7 +3905,7 @@ static int inet_ctl_ifget(inet_descriptor* desc, char* buf, int len, INTERFACE_INFO* ifp; long namaddr; - if ((len == 0) || ((namlen = buf[0]) > len)) + if ((len == 0) || ((namlen = get_int8(buf)) > len)) goto error; if (parse_addr(buf+1, namlen, &namaddr) < 0) goto error; @@ -4099,7 +4099,7 @@ static int inet_ctl_ifget(inet_descriptor* desc, char* buf, int len, struct ifreq ifreq; int namlen; - if ((len == 0) || ((namlen = buf[0]) > len)) + if ((len == 0) || ((namlen = get_int8(buf)) > len)) goto error; sys_memset(ifreq.ifr_name, '\0', IFNAMSIZ); sys_memcpy(ifreq.ifr_name, buf+1, @@ -4252,7 +4252,7 @@ static int inet_ctl_ifset(inet_descriptor* desc, char* buf, int len, int namlen; char* b_end = buf + len; - if ((len == 0) || ((namlen = buf[0]) > len)) + if ((len == 0) || ((namlen = get_int8(buf)) > len)) goto error; sys_memset(ifreq.ifr_name, '\0', IFNAMSIZ); sys_memcpy(ifreq.ifr_name, buf+1, diff --git a/lib/kernel/test/inet_SUITE.erl b/lib/kernel/test/inet_SUITE.erl index eb8f918491..d475ec74b6 100644 --- a/lib/kernel/test/inet_SUITE.erl +++ b/lib/kernel/test/inet_SUITE.erl @@ -26,7 +26,8 @@ t_gethostbyaddr_v6/1, t_getaddr_v6/1, t_gethostbyname_v6/1, ipv4_to_ipv6/1, host_and_addr/1, parse/1, t_gethostnative/1, gethostnative_parallell/1, cname_loop/1, - gethostnative_soft_restart/1,gethostnative_debug_level/1,getif/1]). + gethostnative_soft_restart/1,gethostnative_debug_level/1,getif/1, + getif_ifr_name_overflow/1]). -export([get_hosts/1, get_ipv6_hosts/1, parse_hosts/1, parse_address/1, kill_gethost/0, parallell_gethost/0]). @@ -39,7 +40,7 @@ all(suite) -> ipv4_to_ipv6, host_and_addr, parse,t_gethostnative, gethostnative_parallell, cname_loop, gethostnative_debug_level,gethostnative_soft_restart, - getif]. + getif,getif_ifr_name_overflow]. init_per_testcase(_Func, Config) -> Dog = test_server:timetrap(test_server:seconds(60)), @@ -891,6 +892,13 @@ getif(Config) when is_list(Config) -> ?line true = ip_member(Loopback, Addresses), ?line ok. +getif_ifr_name_overflow(doc) -> + "Test long interface names do not overrun buffer"; +getif_ifr_name_overflow(Config) when is_list(Config) -> + %% emulator should not crash + ?line {ok,[]} = inet:ifget(lists:duplicate(128, "x"), [addr]), + ok. + %% Works just like lists:member/2, except that any {127,_,_,_} tuple %% matches any other {127,_,_,_}. We do this to handle Linux systems %% that use (for instance) 127.0.1.1 as the IP address for the hostname. -- cgit v1.2.3 From d61d41e235bc464a15925e6fd1b7e9138477d3b3 Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Fri, 23 Jul 2010 11:29:20 -0400 Subject: inet: fix getservbyname buffer overflow The byte holding the length of the interface name for the getservbyname/2 function is used in a signed context and can become negative, causing the buffer to be overrun. Make the same change for getservbyport/2. Test case: inet:getservbyname(list_to_atom(lists:flatten(lists:duplicate(128, "x"))), tcp). --- erts/emulator/drivers/common/inet_drv.c | 6 +++--- lib/kernel/test/inet_SUITE.erl | 11 +++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 87dc63509d..8e312c0292 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -6847,13 +6847,13 @@ static int inet_ctl(inet_descriptor* desc, int cmd, char* buf, int len, if (len < 2) return ctl_error(EINVAL, rbuf, rsize); - n = buf[0]; buf++; len--; + n = get_int8(buf); buf++; len--; if (n >= len) /* the = sign makes the test inklude next length byte */ return ctl_error(EINVAL, rbuf, rsize); memcpy(namebuf, buf, n); namebuf[n] = '\0'; len -= n; buf += n; - n = buf[0]; buf++; len--; + n = get_int8(buf); buf++; len--; if (n > len) return ctl_error(EINVAL, rbuf, rsize); memcpy(protobuf, buf, n); @@ -6876,7 +6876,7 @@ static int inet_ctl(inet_descriptor* desc, int cmd, char* buf, int len, port = get_int16(buf); port = sock_htons(port); buf += 2; - n = buf[0]; buf++; len -= 3; + n = get_int8(buf); buf++; len -= 3; if (n > len) return ctl_error(EINVAL, rbuf, rsize); memcpy(protobuf, buf, n); diff --git a/lib/kernel/test/inet_SUITE.erl b/lib/kernel/test/inet_SUITE.erl index d475ec74b6..86c2a0cbca 100644 --- a/lib/kernel/test/inet_SUITE.erl +++ b/lib/kernel/test/inet_SUITE.erl @@ -27,7 +27,7 @@ ipv4_to_ipv6/1, host_and_addr/1, parse/1, t_gethostnative/1, gethostnative_parallell/1, cname_loop/1, gethostnative_soft_restart/1,gethostnative_debug_level/1,getif/1, - getif_ifr_name_overflow/1]). + getif_ifr_name_overflow/1,getservbyname_overflow/1]). -export([get_hosts/1, get_ipv6_hosts/1, parse_hosts/1, parse_address/1, kill_gethost/0, parallell_gethost/0]). @@ -40,7 +40,7 @@ all(suite) -> ipv4_to_ipv6, host_and_addr, parse,t_gethostnative, gethostnative_parallell, cname_loop, gethostnative_debug_level,gethostnative_soft_restart, - getif,getif_ifr_name_overflow]. + getif,getif_ifr_name_overflow,getservbyname_overflow]. init_per_testcase(_Func, Config) -> Dog = test_server:timetrap(test_server:seconds(60)), @@ -899,6 +899,13 @@ getif_ifr_name_overflow(Config) when is_list(Config) -> ?line {ok,[]} = inet:ifget(lists:duplicate(128, "x"), [addr]), ok. +getservbyname_overflow(doc) -> + "Test long service names do not overrun buffer"; +getservbyname_overflow(Config) when is_list(Config) -> + %% emulator should not crash + ?line {error,einval} = inet:getservbyname(list_to_atom(lists:flatten(lists:duplicate(128, "x"))), tcp), + ok. + %% Works just like lists:member/2, except that any {127,_,_,_} tuple %% matches any other {127,_,_,_}. We do this to handle Linux systems %% that use (for instance) 127.0.1.1 as the IP address for the hostname. -- cgit v1.2.3 From 8393c5d5b682f8acbdc0ecf5b0e6f87589a314cb Mon Sep 17 00:00:00 2001 From: Michael Santos Date: Wed, 21 Jul 2010 14:50:58 -0400 Subject: inet: support retrieving MAC address on BSD On systems supporting getaddrinfo(), support looking up the MAC address from inet:ifget/2. The results have the same quirks as with Linux: if the MAC address is longer than 6 bytes (e.g., fw0 under Mac OS X), the address is truncated; if the interface does not have a MAC address (e.g., lo0), an address consisting of 0's is returned. --- erts/configure.in | 3 +++ erts/emulator/drivers/common/inet_drv.c | 40 +++++++++++++++++++++++++++++---- lib/kernel/test/inet_SUITE.erl | 11 +++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/erts/configure.in b/erts/configure.in index 63bf548c89..74cb9533f3 100644 --- a/erts/configure.in +++ b/erts/configure.in @@ -1764,6 +1764,9 @@ AC_CHECK_FUNCS([fdatasync]) dnl Find which C libraries are required to use fdatasync AC_SEARCH_LIBS(fdatasync, [rt]) +AC_CHECK_HEADERS(net/if_dl.h ifaddrs.h) +AC_CHECK_FUNCS([getifaddrs]) + dnl ---------------------------------------------------------------------- dnl Checks for features/quirks in the system that affects Erlang. dnl ---------------------------------------------------------------------- diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 8e312c0292..dfa3814d78 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -48,6 +48,12 @@ #include #endif +#ifdef HAVE_NET_IF_DL_H +#include +#endif +#ifdef HAVE_IFADDRS_H +#include +#endif /* All platforms fail on malloc errors. */ #define FATAL_MALLOC @@ -4089,6 +4095,10 @@ static int inet_ctl_getiflist(inet_descriptor* desc, char** rbuf, int rsize) } +/* FIXME: temporary hack */ +#ifndef IFHWADDRLEN +#define IFHWADDRLEN 6 +#endif static int inet_ctl_ifget(inet_descriptor* desc, char* buf, int len, char** rbuf, int rsize) @@ -4128,6 +4138,32 @@ static int inet_ctl_ifget(inet_descriptor* desc, char* buf, int len, /* raw memcpy (fix include autoconf later) */ sys_memcpy(sptr, (char*)(&ifreq.ifr_hwaddr.sa_data), IFHWADDRLEN); sptr += IFHWADDRLEN; +#elif defined(HAVE_GETIFADDRS) + struct ifaddrs *ifa, *ifp; + int found = 0; + + if (getifaddrs(&ifa) == -1) + goto error; + + for (ifp = ifa; ifp; ifp = ifp->ifa_next) { + if ((ifp->ifa_addr->sa_family == AF_LINK) && + (sys_strcmp(ifp->ifa_name, ifreq.ifr_name) == 0)) { + found = 1; + break; + } + } + + if (found == 0) { + freeifaddrs(ifa); + break; + } + + buf_check(sptr, s_end, 1+IFHWADDRLEN); + *sptr++ = INET_IFOPT_HWADDR; + sys_memcpy(sptr, ((struct sockaddr_dl *)ifp->ifa_addr)->sdl_data + + ((struct sockaddr_dl *)ifp->ifa_addr)->sdl_nlen, IFHWADDRLEN); + freeifaddrs(ifa); + sptr += IFHWADDRLEN; #endif break; } @@ -4240,10 +4276,6 @@ static int inet_ctl_ifget(inet_descriptor* desc, char* buf, int len, return ctl_error(EINVAL, rbuf, rsize); } -/* FIXME: temporary hack */ -#ifndef IFHWADDRLEN -#define IFHWADDRLEN 6 -#endif static int inet_ctl_ifset(inet_descriptor* desc, char* buf, int len, char** rbuf, int rsize) diff --git a/lib/kernel/test/inet_SUITE.erl b/lib/kernel/test/inet_SUITE.erl index 86c2a0cbca..f4f27933a5 100644 --- a/lib/kernel/test/inet_SUITE.erl +++ b/lib/kernel/test/inet_SUITE.erl @@ -877,6 +877,17 @@ getif(Config) when is_list(Config) -> ?line {ok,Address} = inet:getaddr(Hostname, inet), ?line {ok,Loopback} = inet:getaddr("localhost", inet), ?line {ok,Interfaces} = inet:getiflist(), + ?line HWAs = + lists:sort( + lists:foldl( + fun (I, Acc) -> + case inet:ifget(I, [hwaddr]) of + {ok,[{hwaddr,A}]} -> [A|Acc]; + {ok,[]} -> Acc + end + end, [], Interfaces)), + ?line io:format("HWAs = ~p~n", [HWAs]), + ?line length(HWAs) > 0 orelse ?t:fail(no_HWAs), ?line Addresses = lists:sort( lists:foldl( -- cgit v1.2.3