From 6b483da9c102b1650ab103f78f57f9bd7c707781 Mon Sep 17 00:00:00 2001
From: Ingela Anderton Andin <ingela@erlang.org>
Date: Wed, 30 Apr 2014 12:21:10 +0200
Subject: ssl: Add checks to avoid processing of illegal alerts

---
 lib/ssl/src/ssl_alert.erl      | 29 +++++++++++++++++++++++++++--
 lib/ssl/src/ssl_alert.hrl      |  2 ++
 lib/ssl/src/tls_connection.erl | 22 ++++++++--------------
 3 files changed, 37 insertions(+), 16 deletions(-)

(limited to 'lib/ssl')

diff --git a/lib/ssl/src/ssl_alert.erl b/lib/ssl/src/ssl_alert.erl
index db1535b5ec..78dc98bc25 100644
--- a/lib/ssl/src/ssl_alert.erl
+++ b/lib/ssl/src/ssl_alert.erl
@@ -31,7 +31,7 @@
 -include("ssl_record.hrl").
 -include("ssl_internal.hrl").
 
--export([encode/3, alert_txt/1, reason_code/2]).
+-export([encode/3, decode/1, alert_txt/1, reason_code/2]).
 
 %%====================================================================
 %% Internal application API
@@ -41,11 +41,20 @@
 -spec encode(#alert{}, ssl_record:ssl_version(), #connection_states{}) -> 
 		    {iolist(), #connection_states{}}.
 %%
-%% Description: 
+%% Description: Encodes an alert
 %%--------------------------------------------------------------------
 encode(#alert{} = Alert, Version, ConnectionStates) ->
     ssl_record:encode_alert_record(Alert, Version, ConnectionStates).
 
+%%--------------------------------------------------------------------
+-spec decode(binary()) -> [#alert{}] | #alert{}.
+%%
+%% Description: Decode alert(s), will return a singel own alert if peer
+%% sends garbage or too many warning alerts.
+%%--------------------------------------------------------------------
+decode(Bin) ->
+    decode(Bin, [], 0).
+
 %%--------------------------------------------------------------------
 -spec reason_code(#alert{}, client | server) -> closed | {essl, string()}.
 %%
@@ -71,6 +80,22 @@ alert_txt(#alert{level = Level, description = Description, where = {Mod,Line}})
 %%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
+
+%% It is very unlikely that an correct implementation will send more than one alert at the time
+%% So it there is more than 10 warning alerts we consider it an error
+decode(<<?BYTE(Level), ?BYTE(_), _/binary>>, _, N) when Level == ?WARNING, N > ?MAX_ALERTS ->
+    ?ALERT_REC(?FATAL, ?DECODE_ERROR);
+decode(<<?BYTE(Level), ?BYTE(Description), Rest/binary>>, Acc, N) when Level == ?WARNING ->
+    Alert = ?ALERT_REC(Level, Description),
+    decode(Rest, [Alert | Acc], N + 1);
+decode(<<?BYTE(Level), ?BYTE(Description), _Rest/binary>>, Acc, _) when Level == ?FATAL->
+    Alert = ?ALERT_REC(Level, Description),
+    lists:reverse([Alert | Acc]); %% No need to decode rest fatal alert will end the connection
+decode(<<?BYTE(_Level), _/binary>>, _, _) ->
+    ?ALERT_REC(?FATAL, ?ILLEGAL_PARAMETER);
+decode(<<>>, Acc, _) ->
+    lists:reverse(Acc, []).
+
 level_txt(?WARNING) ->
     "Warning:";
 level_txt(?FATAL) ->
diff --git a/lib/ssl/src/ssl_alert.hrl b/lib/ssl/src/ssl_alert.hrl
index 2d1f323085..f4f1d74264 100644
--- a/lib/ssl/src/ssl_alert.hrl
+++ b/lib/ssl/src/ssl_alert.hrl
@@ -104,6 +104,8 @@
 
 -define(ALERT_REC(Level,Desc), #alert{level=Level,description=Desc,where={?FILE, ?LINE}}).
 
+-define(MAX_ALERTS, 10).
+
 %% Alert
 -record(alert, {
 	  level,
diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl
index 930706cde6..32086ff6ce 100644
--- a/lib/ssl/src/tls_connection.erl
+++ b/lib/ssl/src/tls_connection.erl
@@ -362,20 +362,11 @@ encode_handshake(Handshake, Version, ConnectionStates0, Hist0) ->
         ssl_record:encode_handshake(Frag, Version, ConnectionStates0),
     {Encoded, ConnectionStates, Hist}.
 
-
 encode_change_cipher(#change_cipher_spec{}, Version, ConnectionStates) ->
     ssl_record:encode_change_cipher_spec(Version, ConnectionStates).
 
-
-
 decode_alerts(Bin) ->
-    decode_alerts(Bin, []).
-
-decode_alerts(<<?BYTE(Level), ?BYTE(Description), Rest/binary>>, Acc) ->
-    A = ?ALERT_REC(Level, Description),
-    decode_alerts(Rest, [A | Acc]);
-decode_alerts(<<>>, Acc) ->
-    lists:reverse(Acc, []).
+    ssl_alert:decode(Bin).
 
 initial_state(Role, Host, Port, Socket, {SSLOptions, SocketOptions}, User,
 	      {CbModule, DataTag, CloseTag, ErrorTag}) ->
@@ -420,10 +411,13 @@ next_state(Current,_, #alert{} = Alert, #state{negotiated_version = Version} = S
 next_state(_,Next, no_record, State) ->
     {next_state, Next, State, get_timeout(State)};
 
-next_state(_,Next, #ssl_tls{type = ?ALERT, fragment = EncAlerts}, State) ->
-    Alerts = decode_alerts(EncAlerts),
-    handle_alerts(Alerts,  {next_state, Next, State, get_timeout(State)});
-
+next_state(Current, Next, #ssl_tls{type = ?ALERT, fragment = EncAlerts}, #state{negotiated_version = Version} = State) ->
+    case decode_alerts(EncAlerts) of
+	Alerts = [_|_] ->
+	    handle_alerts(Alerts,  {next_state, Next, State, get_timeout(State)});
+	#alert{} = Alert ->
+	    handle_own_alert(Alert, Version, Current, State)
+    end;
 next_state(Current, Next, #ssl_tls{type = ?HANDSHAKE, fragment = Data},
 	   State0 = #state{protocol_buffers =
 			       #protocol_buffers{tls_handshake_buffer = Buf0} = Buffers,
-- 
cgit v1.2.3