[PATCH] inet: fix getservbyname buffer overflow

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


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 72deedf..1a6a9dd 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 = (unsigned char)buf[0]; 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 = (unsigned char)buf[0]; 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 = (unsigned char)buf[0]; 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 d475ec7..86c2a0c 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.
-- 
1.5.6.4



More information about the erlang-patches mailing list