Remove two buffer overflow vulnerabilities in EPMD

3 files changed, 79 insertions, 12 deletions

diff --git a/erts/epmd/src/epmd_int.h b/erts/epmd/src/epmd_int.h
index 5ead553f36..d69d8c93ef 100644
--- a/erts/epmd/src/epmd_int.h
+++ b/erts/epmd/src/epmd_int.h
@@ -1,3 +1,4 @@
+/* -*- c-indent-level: 2; c-continued-statement-offset: 2 -*- */
 /*
  * %CopyrightBegin%
  *
diff --git a/erts/epmd/src/epmd_srv.c b/erts/epmd/src/epmd_srv.c
index 12ebd7f415..9e470e41b0 100644
--- a/erts/epmd/src/epmd_srv.c
+++ b/erts/epmd/src/epmd_srv.c
@@ -270,11 +270,9 @@ static void do_read(EpmdVars *g,Connection *s)
 		 s->fd,val);
 	  dbg_print_buf(g,s->buf,val);
 
-	  /* FIXME: Shouldn't be needed to close down.... */
 	  node_unreg_sock(g,s->fd);
 	  epmd_conn_close(g,s);
 	}
-      /* FIXME: We always close, probably the right thing to do */
       return;
     }
 
@@ -372,6 +370,8 @@ static int do_accept(EpmdVars *g,int listensock)
     return conn_open(g,msgsock);
 }
 
+/* buf is actually one byte larger than bsize,
+   giving place for null termination */
 static void do_request(g, fd, s, buf, bsize)
      EpmdVars *g;
      int fd;
@@ -382,13 +382,6 @@ static void do_request(g, fd, s, buf, bsize)
   char wbuf[OUTBUF_SIZE];	/* Buffer for writing */
   int i;
 
-  /*
-   * Terminate packet as a C string.  Needed for requests received from Erlang
-   * nodes with lower version than R3A.
-   */
-
-  buf[bsize] = '\0';
-
   switch (*buf)
     {
     case EPMD_ALIVE2_REQ:
@@ -398,7 +391,7 @@ static void do_request(g, fd, s, buf, bsize)
 	 in network byte order, and yyyyyy is symname, possibly null
 	 terminated. */
 
-      if (bsize <= 13)
+      if (bsize <= 14) /* at least one character for the node name */
 	{
 	  dbg_printf(g,0,"packet to small for request ALIVE2_REQ (%d)",bsize);
 	  return;
@@ -421,7 +414,17 @@ static void do_request(g, fd, s, buf, bsize)
 	highvsn = get_int16(&buf[5]);
 	lowvsn = get_int16(&buf[7]);
 	namelen = get_int16(&buf[9]);
+	if (namelen + 13 > bsize) {
+	    dbg_printf(g,0,"Node name size error in ALIVE2_REQ");
+	    return;
+	}
 	extralen = get_int16(&buf[11+namelen]);
+
+	if (extralen + namelen + 13 > bsize) {
+	    dbg_printf(g,0,"Extra info size error in ALIVE2_REQ");
+	    return;
+	}
+
 	for (i = 11 ; i < 11 + namelen; i ++)
 	    if (buf[i] == '\000')  {
 		dbg_printf(g,0,"node name contains ascii 0 in ALIVE2_REQ");
@@ -888,6 +891,11 @@ static Node *node_reg2(EpmdVars *g,
       dbg_printf(g,0,"node name is too long (%d) %s", strlen(name), name);
       return NULL;
     }
+  if (extralen > MAXSYMLEN)
+    {
+      dbg_printf(g,0,"extra data is too long (%d) %s", strlen(name), name);
+      return NULL;
+    }
 
   /* Fail if it is already registered */
 
diff --git a/erts/epmd/test/epmd_SUITE.erl b/erts/epmd/test/epmd_SUITE.erl
index 88980fec63..5bcbe96b31 100644
--- a/erts/epmd/test/epmd_SUITE.erl
+++ b/erts/epmd/test/epmd_SUITE.erl
@@ -63,7 +63,9 @@
     alive_req_too_large/1,
 
     returns_valid_empty_extra/1,
-    returns_valid_populated_extra_with_nulls/1
+    returns_valid_populated_extra_with_nulls/1,
+    buffer_overrun_1/1,
+    buffer_overrun_2/1
    ]).
 
 
@@ -121,7 +123,10 @@ all(suite) ->
      alive_req_too_large,
 
      returns_valid_empty_extra,
-     returns_valid_populated_extra_with_nulls
+     returns_valid_populated_extra_with_nulls,
+
+    buffer_overrun_1,
+    buffer_overrun_2
     ].
 
 %%
@@ -705,6 +710,59 @@ returns_valid_populated_extra_with_nulls(Config) when is_list(Config) ->
     ?line ok = close(Sock),
     ok.
 
+
+buffer_overrun_1(suite) ->
+    [];
+buffer_overrun_1(doc) ->
+    ["Test security vulnerability in fake extra lengths in alive2_req"];
+buffer_overrun_1(Config) when is_list(Config) ->
+    ?line ok = epmdrun(),
+    ?line true = alltrue([hostile(N) || N <- lists:seq(1,10000)]),
+    ok.
+buffer_overrun_2(suite) ->
+    [];
+buffer_overrun_2(doc) ->
+    ["Test security vulnerability in fake extra lengths in alive2_req"];
+buffer_overrun_2(Config) when is_list(Config) ->
+    ?line ok = epmdrun(),
+    ?line [false | Rest] = [hostile2(N) || N <- lists:seq(255,10000)],
+    ?line true = alltrue(Rest),
+    ok.
+hostile(N) ->
+    try
+	Bin= <<$x:8,4747:16,$M:8,0:8,5:16,5:16,5:16,"gurka",N:16>>,
+	S = size(Bin),
+	{ok,E}=connect(),
+	gen_tcp:send(E,[<<S:16>>,Bin]),
+	closed = recv(E,1),
+	gen_tcp:close(E),
+	true
+    catch
+	_:_ ->
+	    false
+    end.
+hostile2(N) ->
+    try
+	B2 = list_to_binary(lists:duplicate(N,255)),
+	Bin= <<$x:8,4747:16,$M:8,0:8,5:16,5:16,5:16,"gurka",N:16,B2/binary>>,
+	S = size(Bin),
+	{ok,E}=connect(),
+	gen_tcp:send(E,[<<S:16>>,Bin]),
+	Z = recv(E,2),
+	gen_tcp:close(E),
+	(Z =:= closed) or (Z =:= {ok, [$y,1]})
+    catch
+	_:_ ->
+	    false
+    end.
+
+alltrue([]) ->
+    true;
+alltrue([true|T]) ->
+    alltrue(T);
+alltrue([_|_]) ->
+    false.
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 % Terminate all tests with killing epmd.