From e07937a3efee646e1f0f0c3e5ec9498b39d840d2 Mon Sep 17 00:00:00 2001
From: firest <enlanubo@gmail.com>
Date: Tue, 10 Oct 2023 13:49:36 +0800
Subject: [PATCH] fix(ldap): escape the escape character (\)

---
 .../test/emqx_authn_ldap_SUITE.erl            |  7 +++++-
 .../test/emqx_authn_ldap_bind_SUITE.erl       |  2 +-
 apps/emqx_ldap/src/emqx_ldap.erl              |  8 +++----
 apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl | 23 +++++++++++++++----
 apps/emqx_ldap/test/data/emqx.io.ldif         |  9 ++++++++
 .../emqx_ldap/test/emqx_ldap_filter_SUITE.erl |  4 ++++
 6 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/apps/emqx_auth_ldap/test/emqx_authn_ldap_SUITE.erl b/apps/emqx_auth_ldap/test/emqx_authn_ldap_SUITE.erl
index a4a85b49b..e75a9a617 100644
--- a/apps/emqx_auth_ldap/test/emqx_authn_ldap_SUITE.erl
+++ b/apps/emqx_auth_ldap/test/emqx_authn_ldap_SUITE.erl
@@ -241,7 +241,12 @@ user_seeds() ->
         New(<<"mqttuser0006">>, <<"mqttuser0006">>, {error, user_disabled}),
         %% IsSuperuser
         New(<<"mqttuser0007">>, <<"mqttuser0007">>, {ok, #{is_superuser => true}}),
-        New(<<"mqttuser0008 (test)">>, <<"mqttuser0008 (test)">>, {ok, #{is_superuser => true}})
+        New(<<"mqttuser0008 (test)">>, <<"mqttuser0008 (test)">>, {ok, #{is_superuser => true}}),
+        New(
+            <<"mqttuser0009 \\\\test\\\\">>,
+            <<"mqttuser0009 \\\\test\\\\">>,
+            {ok, #{is_superuser => true}}
+        )
         | Valid
     ].
 
diff --git a/apps/emqx_auth_ldap/test/emqx_authn_ldap_bind_SUITE.erl b/apps/emqx_auth_ldap/test/emqx_authn_ldap_bind_SUITE.erl
index 5773b4f49..a796b8e01 100644
--- a/apps/emqx_auth_ldap/test/emqx_authn_ldap_bind_SUITE.erl
+++ b/apps/emqx_auth_ldap/test/emqx_authn_ldap_bind_SUITE.erl
@@ -235,7 +235,7 @@ user_seeds() ->
         lists:seq(1, 5)
     ),
 
-    Specials = [<<"mqttuser0008 (test)">>],
+    Specials = [<<"mqttuser0008 (test)">>, <<"mqttuser0009 \\\\test\\\\">>],
 
     Valid =
         lists:map(
diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl
index 2db801b9b..94b8992e0 100644
--- a/apps/emqx_ldap/src/emqx_ldap.erl
+++ b/apps/emqx_ldap/src/emqx_ldap.erl
@@ -319,10 +319,8 @@ do_prepare_template([], State) ->
 
 filter_escape(Binary) when is_binary(Binary) ->
     filter_escape(erlang:binary_to_list(Binary));
-filter_escape([$\\ | T]) ->
-    [$\\, $\\ | filter_escape(T)];
 filter_escape([Char | T]) ->
-    case lists:member(Char, filter_control_chars()) of
+    case lists:member(Char, filter_special_chars()) of
         true ->
             [$\\, Char | filter_escape(T)];
         _ ->
@@ -331,5 +329,5 @@ filter_escape([Char | T]) ->
 filter_escape([]) ->
     [].
 
-filter_control_chars() ->
-    [$(, $), $&, $|, $=, $!, $~, $>, $<, $:, $*, $\t, $\n, $\r].
+filter_special_chars() ->
+    [$(, $), $&, $|, $=, $!, $~, $>, $<, $:, $*, $\t, $\n, $\r, $\\].
diff --git a/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl b/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl
index 7c4d62bc9..3b4851fc4 100644
--- a/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl
+++ b/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl
@@ -2,8 +2,8 @@ Definitions.
 
 Control = [()&|!=~><:*]
 White = [\s\t\n\r]+
-StringChars = [^()&|!=~><:*\t\n\r]
-Escape = \\{Control}|\\{White}
+StringChars = [^()&|!=~><:*\t\n\r\\]
+Escape = \\\\|\\{Control}|\\{White}
 String = ({Escape}|{StringChars})+
 
 Rules.
@@ -23,7 +23,7 @@ Rules.
 {White} : skip_token.
 {String} : {token, {string, TokenLine, to_string(TokenChars)}}.
 %% Leex will hang if a composite operation is missing a character
-{Control} : {error, lists:flatten(io_lib:format("Unexpected Tokens:~ts", [TokenChars]))}.
+{Control} : {error, format("Unexpected Tokens:~ts", [TokenChars])}.
 
 Erlang code.
 
@@ -34,4 +34,19 @@ Erlang code.
 %% so after the tokenization we should remove all escape character
 to_string(TokenChars) ->
     String = string:trim(TokenChars),
-    lists:flatten(string:replace(String, "\\", "", all)).
+    trim_escape(String).
+
+%% because of the below situation, we can't directly use the `replace` to trim the escape character
+%%trim_escape([$\\, $\\ | T]) ->
+%%    [$\\ | trim_escape(T)];
+trim_escape([$\\, Char | T]) ->
+    [Char | trim_escape(T)];
+%% the underneath is impossible to occur because it is not valid in the lexer
+%% trim_escape([$\\])
+trim_escape([Char | T]) ->
+    [Char | trim_escape(T)];
+trim_escape([]) ->
+    [].
+
+format(Fmt, Args) ->
+    lists:flatten(io_lib:format(Fmt, Args)).
diff --git a/apps/emqx_ldap/test/data/emqx.io.ldif b/apps/emqx_ldap/test/data/emqx.io.ldif
index 62ed4c3d5..bc9012b00 100644
--- a/apps/emqx_ldap/test/data/emqx.io.ldif
+++ b/apps/emqx_ldap/test/data/emqx.io.ldif
@@ -166,6 +166,15 @@ uid: mqttuser0008 (test)
 isSuperuser: TRUE
 userPassword: {SHA}FCzJLOp66OwsZ9DQzXSxdTd9c0U=
 
+objectClass: top
+dn:uid=mqttuser0009 \\test\\,ou=testdevice,dc=emqx,dc=io
+objectClass: mqttUser
+objectClass: mqttDevice
+objectClass: mqttSecurity
+uid: mqttuser0009 \\test\\
+isSuperuser: TRUE
+userPassword: {SHA}awxXARLqWYx+xy0677D/TLjlyHA=
+
 ## Try to test with base DN 'ou=dashboard,dc=emqx,dc=io'
 ## with a filter ugroup=group1
 ## this should return 2 users in the query and fail the test
diff --git a/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl b/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl
index eb2b6161c..e1aacef88 100644
--- a/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl
+++ b/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl
@@ -235,6 +235,10 @@ t_escape(_Config) ->
     ?assertEqual(
         'or'([equalityMatch("a", "name (1) *")]),
         parse("(|(a=name\\ \\(1\\) \\*))")
+    ),
+    ?assertEqual(
+        'and'([equalityMatch("a", "\\value\\")]),
+        parse("(&(a=\\\\value\\\\))")
     ).
 
 t_value_eql_dn(_Config) ->