From c64e77a08cd065ab9ae9b53590853b4743e1dc9d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Chmielowski?= <pchmielowski@process-one.net>
Date: Thu, 4 May 2017 11:18:16 +0200
Subject: [PATCH] Catch exceptions from acl:add_list in web admin

---
 src/acl.erl                | 58 +++++++++++++++++++++++++-------------
 src/ejabberd_web_admin.erl | 12 ++++++--
 2 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/src/acl.erl b/src/acl.erl
index 3e0617f55..2d848b99b 100644
--- a/src/acl.erl
+++ b/src/acl.erl
@@ -266,24 +266,42 @@ normalize_spec(Spec) ->
     case Spec of
         all -> all;
         none -> none;
-        {acl, N} -> {acl, N};
-        {user, {U, S}} -> {user, {nodeprep(U), nameprep(S)}};
-        {user, U} -> {user, split_user_server(U, fun nodeprep/1, fun nameprep/1)};
-        {shared_group, {G, H}} -> {shared_group, {b(G), nameprep(H)}};
-        {shared_group, G} -> {shared_group, split_user_server(G, fun b/1, fun nameprep/1)};
-        {user_regexp, {UR, S}} -> {user_regexp, {b(UR), nameprep(S)}};
-        {user_regexp, UR} -> {user_regexp, split_user_server(UR, fun b/1, fun nameprep/1)};
-        {node_regexp, {UR, SR}} -> {node_regexp, {b(UR), b(SR)}};
-        {user_glob, {UR, S}} -> {user_glob, {b(UR), nameprep(S)}};
-        {user_glob, UR} -> {user_glob, split_user_server(UR, fun b/1, fun nameprep/1)};
-        {node_glob, {UR, SR}} -> {node_glob, {b(UR), b(SR)}};
-        {server, S} -> {server, nameprep(S)};
-        {resource, R} -> {resource, resourceprep(R)};
-        {server_regexp, SR} -> {server_regexp, b(SR)};
-        {resource_regexp, R} -> {resource_regexp, b(R)};
-        {server_glob, S} -> {server_glob, b(S)};
-        {resource_glob, R} -> {resource_glob, b(R)};
-        {ip, {Net, Mask}} -> {ip, {Net, Mask}};
+	{acl, N} when is_atom(N) ->
+	    {acl, N};
+	{user, {U, S}} when is_binary(U), is_binary(S) ->
+	    {user, {nodeprep(U), nameprep(S)}};
+	{user, U} when is_binary(U) ->
+	    {user, split_user_server(U, fun nodeprep/1, fun nameprep/1)};
+	{shared_group, {G, H}} when is_binary(G), is_binary(H) ->
+	    {shared_group, {b(G), nameprep(H)}};
+	{shared_group, G} when is_binary(G) ->
+	    {shared_group, split_user_server(G, fun b/1, fun nameprep/1)};
+	{user_regexp, {UR, S}} when is_binary(UR), is_binary(S) ->
+	    {user_regexp, {b(UR), nameprep(S)}};
+	{user_regexp, UR} when is_binary(UR) ->
+	    {user_regexp, split_user_server(UR, fun b/1, fun nameprep/1)};
+	{node_regexp, {UR, SR}} when is_binary(UR), is_binary(SR) ->
+	    {node_regexp, {b(UR), b(SR)}};
+	{user_glob, {UR, S}} when is_binary(UR), is_binary(S) ->
+	    {user_glob, {b(UR), nameprep(S)}};
+	{user_glob, UR} when is_binary(UR) ->
+	    {user_glob, split_user_server(UR, fun b/1, fun nameprep/1)};
+	{node_glob, {UR, SR}} when is_binary(UR), is_binary(SR) ->
+	    {node_glob, {b(UR), b(SR)}};
+	{server, S} when is_binary(S) ->
+	    {server, nameprep(S)};
+	{resource, R} when is_binary(R) ->
+	    {resource, resourceprep(R)};
+	{server_regexp, SR} when is_binary(SR) ->
+	    {server_regexp, b(SR)};
+	{resource_regexp, R} when is_binary(R) ->
+	    {resource_regexp, b(R)};
+	{server_glob, S} when is_binary(S) ->
+	    {server_glob, b(S)};
+	{resource_glob, R} when is_binary(R) ->
+	    {resource_glob, b(R)};
+	{ip, {Net, Mask}} when is_binary(Net), is_integer(Mask) ->
+	    {ip, {Net, Mask}};
         {ip, S} ->
             case parse_ip_netmask(b(S)) of
                 {ok, Net, Mask} ->
@@ -291,7 +309,9 @@ normalize_spec(Spec) ->
                 error ->
                     ?INFO_MSG("Invalid network address: ~p", [S]),
                     none
-            end
+	    end;
+	BadVal ->
+	    throw({<<"Invalid acl value">>, BadVal})
     end.
 
 -spec any_rules_allowed(global | binary(), [access_name()],
diff --git a/src/ejabberd_web_admin.erl b/src/ejabberd_web_admin.erl
index f4186b3e5..0106bb9cd 100644
--- a/src/ejabberd_web_admin.erl
+++ b/src/ejabberd_web_admin.erl
@@ -740,7 +740,10 @@ process_admin(Host,
 		  {ok, Tokens, _} ->
 		      case erl_parse:parse_term(Tokens) of
 			{ok, NewACLs} ->
-                            acl:add_list(Host, NewACLs, true);
+			    case catch acl:add_list(Host, NewACLs, true) of
+				ok -> ok;
+				_ -> error
+			    end;
 			_ -> error
 		      end;
 		  _ -> error
@@ -779,8 +782,11 @@ process_admin(Host,
 		case catch acl_parse_query(Host, Query) of
 		  {'EXIT', _} -> error;
 		  NewACLs ->
-		      ?INFO_MSG("NewACLs at ~s: ~p", [Host, NewACLs]),
-		      acl:add_list(Host, NewACLs, true)
+			?INFO_MSG("NewACLs at ~s: ~p", [Host, NewACLs]),
+			case catch acl:add_list(Host, NewACLs, true) of
+			    ok -> ok;
+			    _ -> error
+			end
 		end;
 	    _ -> nothing
 	  end,