Log an error when an LDAP filter is incorrect (EJAB-1395)

5 files changed, 32 insertions, 5 deletions

diff --git a/src/ejabberd_auth_ldap.erl b/src/ejabberd_auth_ldap.erl
index c21811242..7470dbb0f 100644
--- a/src/ejabberd_auth_ldap.erl
+++ b/src/ejabberd_auth_ldap.erl
@@ -398,7 +398,9 @@ parse_options(Host) ->
     UserFilter = case ejabberd_config:get_local_option({ldap_filter, Host}) of
 		     undefined -> SubFilter;
 		     "" -> SubFilter;
-		     F -> "(&" ++ SubFilter ++ F ++ ")"
+		     F ->
+                         eldap_utils:check_filter(F),
+                         "(&" ++ SubFilter ++ F ++ ")"
 		 end,
     SearchFilter = eldap_filter:do_sub(UserFilter, [{"%u", "*"}]),
     LDAPBase = ejabberd_config:get_local_option({ldap_base, Host}),
@@ -411,7 +413,8 @@ parse_options(Host) ->
 	    {DNF, DNFA} ->
 		{DNF, DNFA}
 	end,
-	LocalFilter = ejabberd_config:get_local_option({ldap_local_filter, Host}),
+    eldap_utils:check_filter(DNFilter),
+    LocalFilter = ejabberd_config:get_local_option({ldap_local_filter, Host}),
     #state{host = Host,
 	   eldap_id = Eldap_ID,
 	   bind_eldap_id = Bind_Eldap_ID,
diff --git a/src/eldap/eldap_filter.erl b/src/eldap/eldap_filter.erl
index e1b5be303..d2e1b66a4 100644
--- a/src/eldap/eldap_filter.erl
+++ b/src/eldap/eldap_filter.erl
@@ -82,6 +82,8 @@ parse(L) when is_list(L) ->
 %%%-------------------------------------------------------------------
 parse(L, SList) when is_list(L), is_list(SList) ->
     case catch eldap_filter_yecc:parse(scan(L, SList)) of
+        {'EXIT', _} = Err ->
+            {error, Err};
 	{error, {_, _, Msg}} ->
 	    {error, Msg};
 	{ok, Result} ->
diff --git a/src/eldap/eldap_utils.erl b/src/eldap/eldap_utils.erl
index b8ddee36b..5f3a24283 100644
--- a/src/eldap/eldap_utils.erl
+++ b/src/eldap/eldap_utils.erl
@@ -35,8 +35,11 @@
 	 make_filter/2,
 	 get_state/2,
 	 case_insensitive_match/2,
+         check_filter/1,
 	 uids_domain_subst/2]).
 
+-include("ejabberd.hrl").
+
 %% Generate an 'or' LDAP query on one or several attributes
 %% If there is only one attribute
 generate_subfilter([UID]) ->
@@ -144,3 +147,16 @@ uids_domain_subst(Host, UIDs) ->
                   (A) -> A 
               end,
               UIDs).
+
+check_filter(undefined) ->
+    ok;
+check_filter(Filter) ->
+    case eldap_filter:parse(Filter) of
+        {ok, _} ->
+            ok;
+        Err ->
+            ?ERROR_MSG("failed to parse LDAP filter:~n"
+                       "** Filter: ~p~n"
+                       "** Reason: ~p",
+                       [Filter, Err])
+    end.
diff --git a/src/mod_shared_roster_ldap.erl b/src/mod_shared_roster_ldap.erl
index e2ae9bca9..613371922 100644
--- a/src/mod_shared_roster_ldap.erl
+++ b/src/mod_shared_roster_ldap.erl
@@ -619,7 +619,9 @@ parse_options(Host, Opts) ->
 		       RF ->
 			   RF
 		   end,
-
+    lists:foreach(fun eldap_utils:check_filter/1, 
+                  [ConfigFilter, ConfigUserFilter,
+                   ConfigGroupFilter, RosterFilter]),
     SubFilter = "(&("++UIDAttr++"="++UIDAttrFormat++")("++GroupAttr++"=%g))",
     UserSubFilter = case ConfigUserFilter of
                         undefined -> eldap_filter:do_sub(SubFilter, [{"%g", "*"}]);
diff --git a/src/mod_vcard_ldap.erl b/src/mod_vcard_ldap.erl
index 30c694358..7afd1f210 100644
--- a/src/mod_vcard_ldap.erl
+++ b/src/mod_vcard_ldap.erl
@@ -740,10 +740,14 @@ parse_options(Host, Opts) ->
 			 case ejabberd_config:get_local_option({ldap_filter, Host}) of
 			     undefined -> SubFilter;
 			     "" -> SubFilter;
-			     F -> "(&" ++ SubFilter ++ F ++ ")"
+			     F ->
+                                 eldap_utils:check_filter(F),
+                                 "(&" ++ SubFilter ++ F ++ ")"
 			 end;
 		     "" -> SubFilter;
-		     F -> "(&" ++ SubFilter ++ F ++ ")"
+		     F ->
+                         eldap_utils:check_filter(F),
+                         "(&" ++ SubFilter ++ F ++ ")"
 		 end,
     {ok, SearchFilter} = eldap_filter:parse(
 			   eldap_filter:do_sub(UserFilter, [{"%u","*"}])),