Skip to content
Browse files

Remove two buffer overflow vulnerabilities in EPMD

  • Loading branch information...
1 parent f5be3ae commit 716d3f57b471b2e2c3b5772008f5d32767c6cbeb @bufflig bufflig committed Aug 25, 2010
Showing with 79 additions and 12 deletions.
  1. +1 −0 erts/epmd/src/epmd_int.h
  2. +18 −10 erts/epmd/src/epmd_srv.c
  3. +60 −2 erts/epmd/test/epmd_SUITE.erl
View
1 erts/epmd/src/epmd_int.h
@@ -1,3 +1,4 @@
+/* -*- c-indent-level: 2; c-continued-statement-offset: 2 -*- */
/*
* %CopyrightBegin%
*
View
28 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 */
View
62 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.

0 comments on commit 716d3f5

Please sign in to comment.
Something went wrong with that request. Please try again.