From 614bd9dd723bc7f4d854adfc2ab8b475027297a9 Mon Sep 17 00:00:00 2001
From: Evgeniy Khramtsov <ekhramtsov@process-one.net>
Date: Sat, 9 Dec 2017 22:39:43 +0300
Subject: [PATCH] Better process subtag decoding errors

---
 rebar.config            |  2 +-
 src/ejabberd_c2s.erl    |  7 ++++++-
 src/xmpp_stream_in.erl  |  9 +++++++--
 src/xmpp_stream_out.erl | 10 ++++++++--
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/rebar.config b/rebar.config
index 0859ce3b7..42eab79c6 100644
--- a/rebar.config
+++ b/rebar.config
@@ -25,7 +25,7 @@
         {fast_tls, ".*", {git, "https://github.com/processone/fast_tls", {tag, "1.0.17"}}},
         {stringprep, ".*", {git, "https://github.com/processone/stringprep", {tag, "1.0.10"}}},
         {fast_xml, ".*", {git, "https://github.com/processone/fast_xml", "f22a56d"}},
-        {xmpp, ".*", {git, "https://github.com/processone/xmpp", "320e00a"}},
+        {xmpp, ".*", {git, "https://github.com/processone/xmpp", "597d78b"}},
         {fast_yaml, ".*", {git, "https://github.com/processone/fast_yaml", {tag, "1.0.12"}}},
         {jiffy, ".*", {git, "https://github.com/davisp/jiffy", {tag, "0.14.8"}}},
         {p1_oauth2, ".*", {git, "https://github.com/processone/p1_oauth2", {tag, "0.6.2"}}},
diff --git a/src/ejabberd_c2s.erl b/src/ejabberd_c2s.erl
index e87d307e2..258b5f3f2 100644
--- a/src/ejabberd_c2s.erl
+++ b/src/ejabberd_c2s.erl
@@ -468,11 +468,16 @@ handle_authenticated_packet(Pkt, #{lserver := LServer, jid := JID,
 	drop ->
 	    State2;
 	#iq{type = set, sub_els = [_]} ->
-	    case xmpp:get_subtag(Pkt2, #xmpp_session{}) of
+	    try xmpp:try_subtag(Pkt2, #xmpp_session{}) of
 		#xmpp_session{} ->
 		    send(State2, xmpp:make_iq_result(Pkt2));
 		_ ->
 		    check_privacy_then_route(State2, Pkt2)
+	    catch _:{xmpp_codec, Why} ->
+		    Txt = xmpp:io_format_error(Why),
+		    Lang = maps:get(lang, State),
+		    Err = xmpp:err_bad_request(Txt, Lang),
+		    send_error(State2, Pkt2, Err)
 	    end;
 	#presence{to = #jid{luser = LUser, lserver = LServer,
 			    lresource = <<"">>}} ->
diff --git a/src/xmpp_stream_in.erl b/src/xmpp_stream_in.erl
index 3154c9406..caad482c5 100644
--- a/src/xmpp_stream_in.erl
+++ b/src/xmpp_stream_in.erl
@@ -614,8 +614,8 @@ process_authenticated_packet(Pkt, #{mod := Mod} = State) ->
 
 -spec process_bind(xmpp_element(), state()) -> state().
 process_bind(#iq{type = set, sub_els = [_]} = Pkt,
-	     #{xmlns := ?NS_CLIENT, mod := Mod} = State) ->
-    case xmpp:get_subtag(Pkt, #bind{}) of
+	     #{xmlns := ?NS_CLIENT, mod := Mod, lang := MyLang} = State) ->
+    try xmpp:try_subtag(Pkt, #bind{}) of
 	#bind{resource = R} ->
 	    case Mod:bind(R, State) of
 		{ok, #{user := U, server := S, resource := NewR} = State1}
@@ -632,6 +632,11 @@ process_bind(#iq{type = set, sub_els = [_]} = Pkt,
 		    Err = xmpp:err_not_authorized(),
 		    send_error(State, Pkt, Err)
 	    end
+    catch _:{xmpp_codec, Why} ->
+	    Txt = xmpp:io_format_error(Why),
+	    Lang = select_lang(MyLang, xmpp:get_lang(Pkt)),
+	    Err = xmpp:err_bad_request(Txt, Lang),
+	    send_error(State, Pkt, Err)
     end;
 process_bind(Pkt, #{mod := Mod} = State) ->
     try Mod:handle_unbinded_packet(Pkt, State)
diff --git a/src/xmpp_stream_out.erl b/src/xmpp_stream_out.erl
index 335984975..8f4fa5c84 100644
--- a/src/xmpp_stream_out.erl
+++ b/src/xmpp_stream_out.erl
@@ -522,7 +522,7 @@ process_features(StreamFeatures,
 	false ->
 	    TLSRequired = is_starttls_required(State1),
 	    TLSAvailable = is_starttls_available(State1),
-	    case xmpp:get_subtag(StreamFeatures, #starttls{}) of
+	    try xmpp:try_subtag(StreamFeatures, #starttls{}) of
 		false when TLSRequired and not Encrypted ->
 		    Txt = <<"Use of STARTTLS required">>,
 		    send_pkt(State1, xmpp:serr_policy_violation(Txt, Lang));
@@ -543,14 +543,20 @@ process_features(StreamFeatures,
 		    case is_disconnected(State2) of
 			true -> State2;
 			false ->
-			    case xmpp:get_subtag(StreamFeatures, #sasl_mechanisms{}) of
+			    try xmpp:try_subtag(StreamFeatures, #sasl_mechanisms{}) of
 				#sasl_mechanisms{list = Mechs} ->
 				    process_sasl_mechanisms(Mechs, State2);
 				false ->
 				    process_sasl_failure(
 				      <<"Peer provided no SASL mechanisms">>, State2)
+			    catch _:{xmpp_codec, Why} ->
+				    Txt = xmpp:io_format_error(Why),
+				    process_sasl_failure(Txt, State1)
 			    end
 		    end
+	    catch _:{xmpp_codec, Why} ->
+		    Txt = xmpp:io_format_error(Why),
+		    process_sasl_failure(Txt, State1)
 	    end
     end.