From bb7476d3af104330e3a0191c726616684475f895 Mon Sep 17 00:00:00 2001
From: firest <blankalupo@163.com>
Date: Wed, 2 Nov 2022 14:45:47 +0800
Subject: [PATCH] fix(listeners): Constraints the atom convert when parsing the
 esockd access rules

esockd rules only use words 'allow' and 'deny', both are existing,
so it is better to restrict the conversion and print a log when errors
---
 apps/emqx/src/emqx_listeners.erl              | 32 +++++++++++++------
 apps/emqx/src/emqx_misc.erl                   | 10 +++---
 .../coap/handler/emqx_coap_pubsub_handler.erl |  8 +----
 apps/emqx_gateway/src/emqx_gateway_utils.erl  | 15 ++-------
 4 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl
index 2a809f88d..a0f2b1e7d 100644
--- a/apps/emqx/src/emqx_listeners.erl
+++ b/apps/emqx/src/emqx_listeners.erl
@@ -49,7 +49,8 @@
 -export([
     listener_id/2,
     parse_listener_id/1,
-    ensure_override_limiter_conf/2
+    ensure_override_limiter_conf/2,
+    esockd_access_rules/1
 ]).
 
 -export([pre_config_update/3, post_config_update/5]).
@@ -497,17 +498,28 @@ ip_port({Addr, Port}) ->
     [{ip, Addr}, {port, Port}].
 
 esockd_access_rules(StrRules) ->
-    Access = fun(S) ->
+    Access = fun(S, Acc) ->
         [A, CIDR] = string:tokens(S, " "),
-        {
-            list_to_atom(A),
-            case CIDR of
-                "all" -> all;
-                _ -> CIDR
-            end
-        }
+        %% esockd rules only use words 'allow' and 'deny', both are existing
+        %% comparison of strings may be better, but there is a loss of backward compatibility
+        case emqx_misc:safe_to_existing_atom(A) of
+            {ok, Action} ->
+                [
+                    {
+                        Action,
+                        case CIDR of
+                            "all" -> all;
+                            _ -> CIDR
+                        end
+                    }
+                    | Acc
+                ];
+            _ ->
+                ?SLOG(warning, #{msg => "invalid esockd access rule", rule => S}),
+                Acc
+        end
     end,
-    [Access(R) || R <- StrRules].
+    lists:foldr(Access, [], StrRules).
 
 merge_default(Options) ->
     case lists:keytake(tcp_options, 1, Options) of
diff --git a/apps/emqx/src/emqx_misc.erl b/apps/emqx/src/emqx_misc.erl
index 54157da78..674465e6a 100644
--- a/apps/emqx/src/emqx_misc.erl
+++ b/apps/emqx/src/emqx_misc.erl
@@ -469,9 +469,9 @@ safe_to_existing_atom(In) ->
     safe_to_existing_atom(In, utf8).
 
 safe_to_existing_atom(Bin, Encoding) when is_binary(Bin) ->
-    try_to_existing_atom(fun erlang:binary_to_existing_atom/2, [Bin, Encoding]);
-safe_to_existing_atom(List, _Encoding) when is_list(List) ->
-    try_to_existing_atom(fun erlang:list_to_existing_atom/1, [List]);
+    try_to_existing_atom(fun erlang:binary_to_existing_atom/2, Bin, Encoding);
+safe_to_existing_atom(List, Encoding) when is_list(List) ->
+    try_to_existing_atom(fun(In, _) -> erlang:list_to_existing_atom(In) end, List, Encoding);
 safe_to_existing_atom(Atom, _Encoding) when is_atom(Atom) ->
     {ok, Atom};
 safe_to_existing_atom(_Any, _Encoding) ->
@@ -547,8 +547,8 @@ readable_error_msg(Error) ->
             end
     end.
 
-try_to_existing_atom(Fun, Args) ->
-    try erlang:apply(Fun, Args) of
+try_to_existing_atom(Convert, Data, Encoding) ->
+    try Convert(Data, Encoding) of
         Atom ->
             {ok, Atom}
     catch
diff --git a/apps/emqx_gateway/src/coap/handler/emqx_coap_pubsub_handler.erl b/apps/emqx_gateway/src/coap/handler/emqx_coap_pubsub_handler.erl
index 9c12e3343..8587dc1dc 100644
--- a/apps/emqx_gateway/src/coap/handler/emqx_coap_pubsub_handler.erl
+++ b/apps/emqx_gateway/src/coap/handler/emqx_coap_pubsub_handler.erl
@@ -121,13 +121,7 @@ apply_publish_opts(Msg, MQTTMsg) ->
             maps:fold(
                 fun
                     (<<"retain">>, V, Acc) ->
-                        Val =
-                            case emqx_misc:safe_to_existing_atom(V) of
-                                {ok, true} ->
-                                    true;
-                                _ ->
-                                    false
-                            end,
+                        Val = V =:= <<"true">>,
                         emqx_message:set_flag(retain, Val, Acc);
                     (<<"expiry">>, V, Acc) ->
                         Val = erlang:binary_to_integer(V),
diff --git a/apps/emqx_gateway/src/emqx_gateway_utils.erl b/apps/emqx_gateway/src/emqx_gateway_utils.erl
index 8df7d84c0..68fce7589 100644
--- a/apps/emqx_gateway/src/emqx_gateway_utils.erl
+++ b/apps/emqx_gateway/src/emqx_gateway_utils.erl
@@ -70,6 +70,8 @@
     default_subopts/0
 ]).
 
+-import(emqx_listeners, [esockd_access_rules/1]).
+
 -define(ACTIVE_N, 100).
 -define(DEFAULT_IDLE_TIMEOUT, 30000).
 -define(DEFAULT_GC_OPTS, #{count => 1000, bytes => 1024 * 1024}).
@@ -443,19 +445,6 @@ esockd_opts(Type, Opts0) ->
         end
     ).
 
-esockd_access_rules(StrRules) ->
-    Access = fun(S) ->
-        [A, CIDR] = string:tokens(S, " "),
-        {
-            list_to_atom(A),
-            case CIDR of
-                "all" -> all;
-                _ -> CIDR
-            end
-        }
-    end,
-    [Access(R) || R <- StrRules].
-
 ssl_opts(Name, Opts) ->
     Type =
         case Name of