[PATCH] inet: fix ifr_name buffer overflow

Michael Santos michael.santos@REDACTED
Wed Jul 21 17:14:17 CEST 2010


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 0ea5493..e5024d3 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 = (unsigned char)buf[0]) > 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 = (unsigned char)buf[0]) > 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 = (unsigned char)buf[0]) > 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 eb8f918..d475ec7 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.
-- 
1.5.6.4



More information about the erlang-patches mailing list