From 7305923d122fb4503ef0d24d1a7c3e02926b5d0f Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Wed, 16 Nov 2022 18:25:42 +0100
Subject: [PATCH] fix: bridge name parser should not leak atom

---
 apps/emqx_bridge/src/emqx_bridge_api.erl      |  5 +--
 apps/emqx_bridge/src/emqx_bridge_resource.erl | 40 ++++++++++++++++++-
 .../kafka/emqx_bridge_impl_kafka_producer.erl |  4 +-
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl
index d513fd38d..6b5e307d8 100644
--- a/apps/emqx_bridge/src/emqx_bridge_api.erl
+++ b/apps/emqx_bridge/src/emqx_bridge_api.erl
@@ -48,12 +48,11 @@
         {BridgeType, BridgeName} ->
             EXPR
     catch
-        error:{invalid_bridge_id, Id0} ->
+        throw:{invalid_bridge_id, Reason} ->
             {400,
                 error_msg(
                     'INVALID_ID',
-                    <<"invalid_bridge_id: ", Id0/binary,
-                        ". Bridge Ids must be of format {type}:{name}">>
+                    <<"Invalid bride ID, ", Reason/binary>>
                 )}
     end
 ).
diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl
index bb5f5cc54..46722bc59 100644
--- a/apps/emqx_bridge/src/emqx_bridge_resource.erl
+++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl
@@ -74,8 +74,44 @@ bridge_id(BridgeType, BridgeName) ->
 -spec parse_bridge_id(list() | binary() | atom()) -> {atom(), binary()}.
 parse_bridge_id(BridgeId) ->
     case string:split(bin(BridgeId), ":", all) of
-        [Type, Name] -> {binary_to_atom(Type, utf8), Name};
-        _ -> error({invalid_bridge_id, BridgeId})
+        [Type, Name] ->
+            {to_type_atom(Type), validate_name(Name)};
+        _ ->
+            invalid_bridge_id(
+                <<"should be of forst {type}:{name}, but got ", BridgeId/binary>>
+            )
+    end.
+
+validate_name(Name0) ->
+    Name = unicode:characters_to_list(Name0, utf8),
+    case is_list(Name) andalso Name =/= [] of
+        true ->
+            case lists:all(fun is_id_char/1, Name) of
+                true ->
+                    Name0;
+                false ->
+                    invalid_bridge_id(<<"bad name: ", Name0/binary>>)
+            end;
+        false ->
+            invalid_bridge_id(<<"only 0-9a-zA-Z_-. is allowed in name: ", Name0/binary>>)
+    end.
+
+invalid_bridge_id(Reason) -> throw({?FUNCTION_NAME, Reason}).
+
+is_id_char(C) when C >= $0 andalso C =< $9 -> true;
+is_id_char(C) when C >= $a andalso C =< $z -> true;
+is_id_char(C) when C >= $A andalso C =< $Z -> true;
+is_id_char($_) -> true;
+is_id_char($-) -> true;
+is_id_char($.) -> true;
+is_id_char(_) -> false.
+
+to_type_atom(Type) ->
+    try
+        erlang:binary_to_existing_atom(Type, utf8)
+    catch
+        _:_ ->
+            invalid_bridge_id(<<"unknown type: ", Type/binary>>)
     end.
 
 reset_metrics(ResourceId) ->
diff --git a/lib-ee/emqx_ee_bridge/src/kafka/emqx_bridge_impl_kafka_producer.erl b/lib-ee/emqx_ee_bridge/src/kafka/emqx_bridge_impl_kafka_producer.erl
index aceafd6d1..0eeb9db0c 100644
--- a/lib-ee/emqx_ee_bridge/src/kafka/emqx_bridge_impl_kafka_producer.erl
+++ b/lib-ee/emqx_ee_bridge/src/kafka/emqx_bridge_impl_kafka_producer.erl
@@ -266,7 +266,9 @@ make_client_id(BridgeName) ->
 make_producer_name(BridgeName) when is_atom(BridgeName) ->
     make_producer_name(atom_to_list(BridgeName));
 make_producer_name(BridgeName) ->
-    list_to_atom("kafka_producer_" ++ BridgeName).
+    %% Woff needs atom for ets table name registration
+    %% The assumption here is bridge is not often re-created
+    binary_to_atom(iolist_to_binary(["kafka_producer_", BridgeName])).
 
 with_log_at_error(Fun, Log) ->
     try