fix: Add string legitimacy check.

apps/emqx_management/src/emqx_mgmt_auth.erl

@@ -89,36 +89,45 @@ add_app(AppId, Name, Desc, Status, Expired) when is_binary(AppId) ->
       -> {ok, appsecret()}
        | {error, term()}).
 add_app(AppId, Name, Secret, Desc, Status, Expired) when is_binary(AppId) ->
-    Secret1 = generate_appsecret_if_need(Secret),
+    case emqx_misc:valid_str(Name) of
-    App = #mqtt_app{id = AppId,
+        ok ->
-                    secret = Secret1,
+            Secret1 = generate_appsecret_if_need(Secret),
-                    name = Name,
+            App = #mqtt_app{id = AppId,
-                    desc = Desc,
+                            secret = Secret1,
-                    status = Status,
+                            name = Name,
-                    expired = Expired},
+                            desc = Desc,
-    AddFun = fun() ->
+                            status = Status,
-                 case mnesia:wread({mqtt_app, AppId}) of
+                            expired = Expired},
-                     [] -> mnesia:write(App);
+            AddFun = fun() ->
-                     _  -> mnesia:abort(alread_existed)
+                case mnesia:wread({mqtt_app, AppId}) of
-                 end
+                    [] -> mnesia:write(App);
-             end,
+                    _  -> mnesia:abort(alread_existed)
-    case mnesia:transaction(AddFun) of
+                end
-        {atomic, ok} -> {ok, Secret1};
+                     end,
-        {aborted, Reason} -> {error, Reason}
+            case mnesia:transaction(AddFun) of
+                {atomic, ok} -> {ok, Secret1};
+                {aborted, Reason} -> {error, Reason}
+            end;
+        {error, Reason} -> {error, Reason}
     end.
 
 force_add_app(AppId, Name, Secret, Desc, Status, Expired) ->
-    AddFun = fun() ->
+    case emqx_misc:valid_str(Name) of
-                 mnesia:write(#mqtt_app{id = AppId,
+        ok ->
-                                        secret = Secret,
+            AddFun = fun() ->
-                                        name = Name,
+                mnesia:write(#mqtt_app{
-                                        desc = Desc,
+                    id = AppId,
-                                        status = Status,
+                    secret = Secret,
-                                        expired = Expired})
+                    name = Name,
-             end,
+                    desc = Desc,
-    case mnesia:transaction(AddFun) of
+                    status = Status,
-        {atomic, ok} -> ok;
+                    expired = Expired})
-        {aborted, Reason} -> {error, Reason}
+                     end,
+            case mnesia:transaction(AddFun) of
+                {atomic, ok} -> ok;
+                {aborted, Reason} -> {error, Reason}
+            end;
+        {error, Reason} -> {error, Reason}
     end.
 
 -spec(generate_appsecret_if_need(binary() | undefined) -> binary()).
@@ -207,4 +216,3 @@ is_authorized(AppId, AppSecret) ->
 
 is_expired(undefined) -> true;
 is_expired(Expired)   -> Expired >= erlang:system_time(second).
-

lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -78,18 +78,24 @@ start_link() ->
 
 -spec(add_user(binary(), binary(), binary()) -> ok | {error, any()}).
 add_user(Username, Password, Tags) when is_binary(Username), is_binary(Password) ->
-    Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags},
+    case emqx_misc:valid_str(Username) of
-    return(mnesia:transaction(fun add_user_/1, [Admin])).
+        ok ->
+            Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags},
+            return(mnesia:transaction(fun add_user_/1, [Admin]));
+        {error, Reason} -> {error, Reason}
+    end.
 
 force_add_user(Username, Password, Tags) ->
-    AddFun = fun() ->
+    case emqx_misc:valid_str(Username) of
-                 mnesia:write(#mqtt_admin{username = Username,
+        ok ->
-                                          password = Password,
+            AddFun = fun() ->
-                                          tags = Tags})
+                mnesia:write(#mqtt_admin{username = Username, password = Password, tags = Tags})
-             end,
+                     end,
-    case mnesia:transaction(AddFun) of
+            case mnesia:transaction(AddFun) of
-        {atomic, ok} -> ok;
+                {atomic, ok} -> ok;
-        {aborted, Reason} -> {error, Reason}
+                {aborted, Reason} -> {error, Reason}
+            end;
+        {error, Reason} -> {error, Reason}
     end.
 
 %% @private

lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl

@@ -67,6 +67,9 @@ t_overview(_) ->
 t_admins_add_delete(_) ->
     ok = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, <<"tag">>),
     ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag1">>),
+    {error, _} = emqx_dashboard_admin:add_user(<<"1username1">>, <<"password1">>, <<"tag1">>),
+    {error, _} = emqx_dashboard_admin:add_user(<<"u/sername1">>, <<"password1">>, <<"tag1">>),
+    {error, _} = emqx_dashboard_admin:add_user(<<"/username1">>, <<"password1">>, <<"tag1">>),
     Admins = emqx_dashboard_admin:all_users(),
     ?assertEqual(3, length(Admins)),
     ok = emqx_dashboard_admin:remove_user(<<"username1">>),
@@ -165,4 +168,3 @@ api_path(Path) ->
 
 json(Data) ->
     {ok, Jsx} = emqx_json:safe_decode(Data, [return_maps]), Jsx.
-

src/emqx_misc.erl

@@ -52,6 +52,26 @@
         , hexstr2bin/1
         ]).
 
+-export([ valid_str/1
+        ]).
+
+-define(VALID_STR_RE, "^[A-Za-z]+[A-Za-z0-9-_]*$").
+
+-spec valid_str(list() | binary()) -> ok | {error, Reason::binary()}.
+valid_str(Str) ->
+    StrLen = len(Str),
+    case StrLen > 0 andalso StrLen =< 256 of
+        true ->
+            case re:run(Str, ?VALID_STR_RE) of
+                nomatch -> {error, <<"required: " ?VALID_STR_RE>>};
+                _ -> ok
+            end;
+        false -> {error, <<"0 < Length =< 256">>}
+    end.
+
+len(Bin) when is_binary(Bin) -> byte_size(Bin);
+len(Str) when is_list(Str) -> length(Str).
+
 -define(OOM_FACTOR, 1.25).
 
 %% @doc Parse v4 or v6 string format address to tuple.
@@ -309,4 +329,29 @@ hexchar2int(I) when I >= $a andalso I =< $f -> I - $a + 10.
 ipv6_probe_test() ->
     ?assertEqual([{ipv6_probe, true}], ipv6_probe([])).
 
+valid_str_test() ->
+    ?assertMatch({error, _}, valid_str("")),
+    ?assertMatch({error, _}, valid_str("_")),
+    ?assertMatch({error, _}, valid_str("_aaa")),
+    ?assertMatch({error, _}, valid_str("lkad/oddl")),
+    ?assertMatch({error, _}, valid_str("lkad*oddl")),
+    ?assertMatch({error, _}, valid_str("<script>lkadoddl")),
+    ?assertMatch({error, _}, valid_str("1lkdfaldk")),
+    ?assertMatch({error, _}, valid_str("1223333434")),
+
+    ?assertMatch(ok, valid_str(<<"Abckdf_lkdfd_1222">>)),
+    ?assertMatch(ok, valid_str("Abckdf_lkdfd_1222")),
+    ?assertMatch(ok, valid_str("abckdf_lkdfd_1222")),
+    ?assertMatch(ok, valid_str("abckdflkdfd1222")),
+    ?assertMatch(ok, valid_str("abckdflkdf")),
+    ?assertMatch(ok, valid_str("a1122222")),
+
+    Ok = lists:flatten(lists:duplicate(256, "a")),
+    Bad = Ok ++ "a",
+    ?assertMatch(ok, valid_str(Ok)),
+    ?assertMatch(ok, valid_str(list_to_binary(Ok))),
+    ?assertMatch({error, _}, valid_str(Bad)),
+    ?assertMatch({error, _}, valid_str(list_to_binary(Bad))),
+    ok.
+
 -endif.