diff --git a/apps/emqx/src/emqx_misc.erl b/apps/emqx/src/emqx_misc.erl index 6833cbabb..54157da78 100644 --- a/apps/emqx/src/emqx_misc.erl +++ b/apps/emqx/src/emqx_misc.erl @@ -52,7 +52,9 @@ explain_posix/1, pmap/2, pmap/3, - readable_error_msg/1 + readable_error_msg/1, + safe_to_existing_atom/1, + safe_to_existing_atom/2 ]). -export([ @@ -463,6 +465,18 @@ nolink_apply(Fun, Timeout) when is_function(Fun, 0) -> exit(timeout) end. +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]); +safe_to_existing_atom(Atom, _Encoding) when is_atom(Atom) -> + {ok, Atom}; +safe_to_existing_atom(_Any, _Encoding) -> + {error, invalid_type}. + %%------------------------------------------------------------------------------ %% Internal Functions %%------------------------------------------------------------------------------ @@ -533,6 +547,14 @@ readable_error_msg(Error) -> end end. +try_to_existing_atom(Fun, Args) -> + try erlang:apply(Fun, Args) of + Atom -> + {ok, Atom} + catch + _:Reason -> {error, Reason} + end. + -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 37a42ab3d..66a9079f8 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -492,23 +492,16 @@ lookup_from_local_node(BridgeType, BridgeName) -> invalid -> {400, error_msg('BAD_REQUEST', <<"invalid operation">>)}; OperFunc -> - TargetNode = binary_to_atom(Node, utf8), ConfMap = emqx:get_config([bridges, BridgeType, BridgeName]), case maps:get(enable, ConfMap, false) of false -> {403, error_msg( - 'FORBIDDEN_REQUEST', <<"forbidden operation: bridge disabled">> + 'FORBIDDEN_REQUEST', + <<"forbidden operation: bridge disabled">> )}; true -> - case emqx_bridge_proto_v1:OperFunc(TargetNode, BridgeType, BridgeName) of - ok -> - {200}; - {error, timeout} -> - {503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)}; - {error, Reason} -> - {500, error_msg('INTERNAL_ERROR', Reason)} - end + call_operation(Node, OperFunc, BridgeType, BridgeName) end end ). @@ -707,3 +700,22 @@ bin(S) when is_atom(S) -> atom_to_binary(S, utf8); bin(S) when is_binary(S) -> S. + +call_operation(Node, OperFunc, BridgeType, BridgeName) -> + case emqx_misc:safe_to_existing_atom(Node, utf8) of + {ok, TargetNode} -> + case + emqx_bridge_proto_v1:OperFunc( + TargetNode, BridgeType, BridgeName + ) + of + ok -> + {200}; + {error, timeout} -> + {503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)}; + {error, Reason} -> + {500, error_msg('INTERNAL_ERROR', Reason)} + end; + {error, _} -> + {400, error_msg('INVALID_NODE', <<"invalid node">>)} + end. diff --git a/apps/emqx_bridge/src/emqx_bridge_resource.erl b/apps/emqx_bridge/src/emqx_bridge_resource.erl index d19cc8426..cbfeda4ba 100644 --- a/apps/emqx_bridge/src/emqx_bridge_resource.erl +++ b/apps/emqx_bridge/src/emqx_bridge_resource.erl @@ -59,9 +59,10 @@ bridge_id(BridgeType, BridgeName) -> Type = bin(BridgeType), <>. +-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), binary_to_atom(Name, utf8)}; + [Type, Name] -> {binary_to_atom(Type, utf8), Name}; _ -> error({invalid_bridge_id, BridgeId}) end. diff --git a/apps/emqx_connector/src/emqx_connector.erl b/apps/emqx_connector/src/emqx_connector.erl index d85959698..1ce4d982a 100644 --- a/apps/emqx_connector/src/emqx_connector.erl +++ b/apps/emqx_connector/src/emqx_connector.erl @@ -78,9 +78,10 @@ connector_id(Type0, Name0) -> Name = bin(Name0), <>. +-spec parse_connector_id(binary() | list() | atom()) -> {atom(), binary()}. parse_connector_id(ConnectorId) -> case string:split(bin(ConnectorId), ":", all) of - [Type, Name] -> {binary_to_atom(Type, utf8), binary_to_atom(Name, utf8)}; + [Type, Name] -> {binary_to_atom(Type, utf8), Name}; _ -> error({invalid_connector_id, ConnectorId}) end. diff --git a/apps/emqx_dashboard/src/emqx_dashboard_monitor_api.erl b/apps/emqx_dashboard/src/emqx_dashboard_monitor_api.erl index e3ea870af..50349bc40 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_monitor_api.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_monitor_api.erl @@ -131,12 +131,20 @@ monitor(get, #{query_string := QS, bindings := Bindings}) -> end. monitor_current(get, #{bindings := Bindings}) -> - NodeOrCluster = binary_to_atom(maps:get(node, Bindings, <<"all">>), utf8), - case emqx_dashboard_monitor:current_rate(NodeOrCluster) of - {ok, CurrentRate} -> - {200, CurrentRate}; - {badrpc, {Node, Reason}} -> - Message = list_to_binary(io_lib:format("Bad node ~p, rpc failed ~p", [Node, Reason])), + RawNode = maps:get(node, Bindings, all), + case emqx_misc:safe_to_existing_atom(RawNode, utf8) of + {ok, NodeOrCluster} -> + case emqx_dashboard_monitor:current_rate(NodeOrCluster) of + {ok, CurrentRate} -> + {200, CurrentRate}; + {badrpc, {Node, Reason}} -> + Message = list_to_binary( + io_lib:format("Bad node ~p, rpc failed ~p", [Node, Reason]) + ), + {400, 'BAD_RPC', Message} + end; + {error, _} -> + Message = list_to_binary(io_lib:format("Bad node ~p", [RawNode])), {400, 'BAD_RPC', Message} end. 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 2e962a0bc..9c12e3343 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,7 +121,13 @@ apply_publish_opts(Msg, MQTTMsg) -> maps:fold( fun (<<"retain">>, V, Acc) -> - Val = erlang:binary_to_atom(V), + Val = + case emqx_misc:safe_to_existing_atom(V) of + {ok, true} -> + true; + _ -> + false + end, 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_api_clients.erl b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl index 2d6275c62..5f6cc25b6 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl @@ -115,15 +115,19 @@ clients(get, #{ ?QUERY_FUN ); Node0 -> - Node1 = binary_to_atom(Node0, utf8), - QStringWithoutNode = maps:without([<<"node">>], QString), - emqx_mgmt_api:node_query( - Node1, - QStringWithoutNode, - TabName, - ?CLIENT_QSCHEMA, - ?QUERY_FUN - ) + case emqx_misc:safe_to_existing_atom(Node0) of + {ok, Node1} -> + QStringWithoutNode = maps:without([<<"node">>], QString), + emqx_mgmt_api:node_query( + Node1, + QStringWithoutNode, + TabName, + ?CLIENT_QSCHEMA, + ?QUERY_FUN + ); + {error, _} -> + {error, Node0, {badrpc, <<"invalid node">>}} + end end, case Result of {error, page_limit_invalid} -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index 19bf63f66..beff0d53e 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -648,15 +648,19 @@ list_clients(QString) -> ?QUERY_FUN ); Node0 -> - Node1 = binary_to_atom(Node0, utf8), - QStringWithoutNode = maps:without([<<"node">>], QString), - emqx_mgmt_api:node_query( - Node1, - QStringWithoutNode, - ?CLIENT_QTAB, - ?CLIENT_QSCHEMA, - ?QUERY_FUN - ) + case emqx_misc:safe_to_existing_atom(Node0) of + {ok, Node1} -> + QStringWithoutNode = maps:without([<<"node">>], QString), + emqx_mgmt_api:node_query( + Node1, + QStringWithoutNode, + ?CLIENT_QTAB, + ?CLIENT_QSCHEMA, + ?QUERY_FUN + ); + {error, _} -> + {error, Node0, {badrpc, <<"invalid node">>}} + end end, case Result of {error, page_limit_invalid} -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl index 470242cfd..03b833e84 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl @@ -145,13 +145,18 @@ subscriptions(get, #{query_string := QString}) -> ?QUERY_FUN ); Node0 -> - emqx_mgmt_api:node_query( - binary_to_atom(Node0, utf8), - QString, - ?SUBS_QTABLE, - ?SUBS_QSCHEMA, - ?QUERY_FUN - ) + case emqx_misc:safe_to_existing_atom(Node0) of + {ok, Node1} -> + emqx_mgmt_api:node_query( + Node1, + QString, + ?SUBS_QTABLE, + ?SUBS_QSCHEMA, + ?QUERY_FUN + ); + {error, _} -> + {error, Node0, {badrpc, <<"invalid node">>}} + end end, case Response of {error, page_limit_invalid} -> diff --git a/changes/v5.0.10-en.md b/changes/v5.0.10-en.md index 5b1dd5c77..f9ee82a48 100644 --- a/changes/v5.0.10-en.md +++ b/changes/v5.0.10-en.md @@ -15,6 +15,7 @@ - Now it is possible to opt out VM internal metrics in prometheus stats [#9222](https://github.com/emqx/emqx/pull/9222). When system load is high, reporting too much metrics data may cause the prometheus stats API timeout. +- Improve security when converting types such as `binary` `lists` to `atom` types [#9279](https://github.com/emqx/emqx/pull/9279). ## Bug fixes diff --git a/changes/v5.0.10-zh.md b/changes/v5.0.10-zh.md index 2e058e644..623181497 100644 --- a/changes/v5.0.10-zh.md +++ b/changes/v5.0.10-zh.md @@ -14,6 +14,7 @@ - 可通过配置关闭 prometheus 中的部分内部指标，如果遇到机器负载过高 prometheus 接口返回超时可考虑关闭部分不关心指标，以提高响应速度 [#9222](https://github.com/emqx/emqx/pull/9222）。 +- 提升 `binary` 、`list` 等类型转换为 `atom` 类型时的安全性 [#9279](https://github.com/emqx/emqx/pull/9279)。 ## Bug fixes