Merge pull request #10 from esl/handle_bad_base64

Handle gracefully invalid base64 text

rebar.config

@@ -11,7 +11,7 @@
       {base16, "2.0.1"}
      ]},
     {plugins, [
-       {rebar3_codecov, "0.3.0"}
+       {rebar3_codecov, "0.4.0"}
      ]}
    ]},
   {prod, [

src/fast_scram_parse_rules.erl

@@ -84,9 +84,14 @@ parse_nonce(_, _) ->
 -spec parse_salt(binary(), fast_scram_state()) -> parse_return().
 parse_salt(<<>>, _State) ->
     {error, <<"no-resources">>};
-parse_salt(<<"s=", Salt/binary>>, State) ->
+parse_salt(<<"s=", Salt0/binary>>, State) ->
     Challenge = State#fast_scram_state.challenge,
-    {ok, State#fast_scram_state{challenge = Challenge#challenge{salt = base64:decode(Salt)}}};
+    case maybe_base64_decode(Salt0) of
+        {error, Reason} ->
+            {error, Reason};
+        Salt ->
+            {ok, State#fast_scram_state{challenge = Challenge#challenge{salt = Salt}}}
+    end;
 parse_salt(_, _) ->
     {error, <<"other-error">>}.
 
@@ -123,8 +128,12 @@ parse_proof(<<"p=">>, _State) ->
     {error, <<"invalid-proof">>};
 parse_proof(<<"p=", Proof0/binary>>,
             #fast_scram_state{data = Data} = State) ->
-    Proof = base64:decode(Proof0),
-    {ok, State#fast_scram_state{data = Data#{client_proof => Proof}}}.
+    case maybe_base64_decode(Proof0) of
+        {error, Reason} ->
+            {error, Reason};
+        Proof ->
+            {ok, State#fast_scram_state{data = Data#{client_proof => Proof}}}
+    end.
 
 -spec parse_channel_binding(binary(), fast_scram_state()) -> parse_return().
 parse_channel_binding(<<>>, _State) ->
@@ -145,9 +154,11 @@ parse_server_error_or_verifier(
   <<"v=", Verifier/binary>>,
   #fast_scram_state{scram_definitions = #scram_definitions{} = ScramDefs} = State) ->
     ServerSignature = ScramDefs#scram_definitions.server_signature,
-    case base64:decode(Verifier) of
+    case maybe_base64_decode(Verifier) of
         ServerSignature ->
             {ok, State};
+        {error, Reason} ->
+            {error, Reason};
         _ ->
             {error, <<"authentication-failure">>}
     end;
@@ -244,3 +255,10 @@ verify_cbind_input(CBindInput, #channel_binding{data = CBindData} = CBConfig, Da
         true -> ok;
         false -> {error, <<"channel-bindings-dont-match">>}
     end.
+
+maybe_base64_decode(Binary) ->
+    try base64:decode(Binary) of
+        Decoded -> Decoded
+    catch error:badarg ->
+              {error, <<"invalid-encoding">>}
+    end.

test/scram_SUITE.erl

@@ -23,6 +23,7 @@
          verification_name_does_not_escape_values_correctly/1,
          authentication_server_last_message_is_an_error/1,
          authentication_server_rejects_the_proof/1,
+         authentication_server_rejects_invalid_encoded_proof/1,
          authentication_client_rejects_the_signature/1,
          nonce_client_receives_invalid/1,
          nonce_server_finds_non_matching/1,
@@ -89,6 +90,7 @@ groups() ->
       [
        authentication_server_last_message_is_an_error,
        authentication_server_rejects_the_proof,
+       authentication_server_rejects_invalid_encoded_proof,
        authentication_client_rejects_the_signature
       ]},
      {nonce, [parallel],
@@ -316,6 +318,14 @@ authentication_server_rejects_the_proof(_Config) ->
     {error, Reason, _} = fast_scram:mech_step(ServerState4, WrongProof),
     ?assertEqual(<<"e=invalid-proof">>, Reason).
 
+authentication_server_rejects_invalid_encoded_proof(_Config) ->
+    ServerState2 = typical_scram_configuration(server),
+    {continue, _, ServerState4} = fast_scram:mech_step(ServerState2, client_first()),
+    WrongProof = <<"c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,",
+                   "p=wrong_proof">>,
+    {error, Reason, _} = fast_scram:mech_step(ServerState4, WrongProof),
+    ?assertEqual(<<"e=invalid-encoding">>, Reason).
+
 authentication_client_rejects_the_signature(_Config) ->
     ClientState1 = typical_scram_configuration(client),
     {continue, _, ClientState3} = fast_scram:mech_step(ClientState1, <<>>),