Filter the redirect args of logon/logoff to ensure that it is a local…

… redirect (#3096)

* Filter the redirect args of logon/logoff to ensure that it is a local redirect

* Rename local_url to site_url (because master has a is_site_url function).

* Also sanitize fragments.

* mod_translation: sanitize redirect location

* Sanitize more redirect urls in controller_logon

modules/mod_authentication/controllers/controller_logoff.erl

@@ -56,7 +56,7 @@ moved_temporarily(ReqData, Context) ->
     Context1 = ?WM_REQ(ReqData, Context),
     Context2 = z_context:ensure_qs(Context1),
     Location = z_context:get_q("p", Context2, "/"),
-    ?WM_REPLY({true, Location}, Context2).
+    ?WM_REPLY({true, z_context:site_url(Location, Context2)}, Context2).
 
 provide_content(ReqData, Context) ->
     Context1 = ?WM_REQ(ReqData, Context),

modules/mod_authentication/controllers/controller_logon.erl

@@ -91,7 +91,7 @@ previously_existed(ReqData, Context) ->
 %% the user was already logged on and we don't have a redirect page.
 moved_temporarily(ReqData, Context) ->
     Context1 = ?WM_REQ(ReqData, Context),
-    Location = z_context:abs_url(cleanup_url(get_page(Context1)), Context1),
+    Location = z_context:site_url(get_page(Context1), Context1),
     ?WM_REPLY({true, Location}, Context1).
 
 
@@ -101,7 +101,7 @@ provide_content(ReqData, Context) ->
     Vars = [
         {noindex, true},
         {notrack, true},
-        {page, get_page(Context2)}
+        {page, safe_url(get_page(Context2), Context2)}
         | z_context:get_all(Context2)
     ],
     Secret = z_context:get_q("secret", Context2),
@@ -119,6 +119,10 @@ provide_content(ReqData, Context) ->
     {Output, OutputContext} = z_context:output(Rendered, ContextVerify),
     ?WM_REPLY(Output, OutputContext).
 
+safe_url(undefined, _Context) -> undefined;
+safe_url("", _Context) -> "";
+safe_url(<<>>, _Context) -> "";
+safe_url(Url, Context) -> z_context:site_url(Url, Context).
 
 reminder_secrets(undefined, _Username, Context) ->
     {[], Context};
@@ -184,23 +188,23 @@ reset_vars(Context) ->
 %%      This location will be stored in the logon form ("page" on submit).
 get_page(Context) ->
     HasBackArg = z_convert:to_bool(z_context:get_q("back", Context)),
-    case z_context:get_q("p", Context, []) of
-        [] when HasBackArg ->
+    case z_context:get_q("p", Context, "") of
+        "" when HasBackArg ->
             RD = z_context:get_reqdata(Context),
             case wrq:get_req_header("referer", RD) of
-                undefined -> [];
-                Referrer -> z_html:noscript(Referrer)
+                undefined -> "";
+                Referrer -> Referrer
             end;
         Other ->
             Other
     end.
 
 %% @doc User logged on, fetch the location of the next page to show
 get_ready_page(Context) ->
-    get_ready_page(z_context:get_q("page", Context, []), Context).
+    get_ready_page(z_context:get_q("page", Context, ""), Context).
 
 get_ready_page(undefined, Context) ->
-    get_ready_page([], Context);
+    get_ready_page("", Context);
 get_ready_page(Page, Context) when is_binary(Page) ->
     get_ready_page(z_convert:to_list(Page), Context);
 get_ready_page(Page, Context) when is_list(Page) ->
@@ -210,11 +214,6 @@ get_ready_page(Page, Context) when is_list(Page) ->
     end.
 
 
-cleanup_url(undefined) -> "/";
-cleanup_url([]) -> "/";
-cleanup_url(Url) -> z_html:noscript(Url).
-
-
 %% @doc Handle the submit of the logon form, this will be handed over to the
 %% different authentication handlers.
 
@@ -271,8 +270,16 @@ event(#submit{message={logon_tos_agree, WireArgs}}, Context) ->
 event(#z_msg_v1{data=Data}, Context) when is_list(Data) ->
     case proplists:get_value(<<"msg">>, Data) of
         <<"logon_redirect">> ->
-            Location = get_ready_page(proplists:get_value(<<"page">>, Data, []), Context),
-            z_render:wire({redirect, [{location, cleanup_url(Location)}]}, Context);
+            Action = case get_ready_page(proplists:get_value(<<"page">>, Data, []), Context) of
+                "#reload" ->
+                    [{reload, []}];
+                <<"#reload">> ->
+                    [{reload, []}];
+                Location ->
+                    LocalUrl = z_context:site_url(Location, Context),
+                    [{redirect, [{location, LocalUrl}]}]
+            end,
+            z_render:wire(Action, Context);
         Msg ->
             lager:warning("controller_logon: unknown msg: ~p", [Msg]),
             Context
@@ -827,9 +834,14 @@ get_by_reminder_secret(Code, Context) ->
 get_post_logon_actions(WireArgs, Context) ->
     case z_notifier:foldl(#logon_actions{args=WireArgs}, [], Context) of
         [] ->
-            case cleanup_url(get_ready_page(Context)) of
-                "#reload" -> [{reload, []}];
-                Location -> [{redirect, [{location, Location}]}]
+            case get_ready_page(Context) of
+                "#reload" ->
+                    [{reload, []}];
+                <<"#reload">> ->
+                    [{reload, []}];
+                Location ->
+                    LocalUrl = z_context:site_url(Location, Context),
+                    [{redirect, [{location, LocalUrl}]}]
             end;
         Actions ->
             Actions

modules/mod_base/controllers/controller_user_agent_select.erl

@@ -1,8 +1,8 @@
 %% @author Marc Worrell <marc@worrell.nl>
-%% @copyright 2012 Marc Worrell <marc@worrell.nl>
+%% @copyright 2012-2022 Marc Worrell <marc@worrell.nl>
 %% @doc Change the selected user agent, by letting him choose from a form/links etc.
 
-%% Copyright 2012 Marc Worrell
+%% Copyright 2012-2022 Marc Worrell
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -62,18 +62,14 @@ previously_existed(ReqData, Context) ->
 
 moved_temporarily(ReqData, Context) ->
     C1 = z_context:set_nocache_headers(?WM_REQ(ReqData, Context)),
-    Loc = case z_context:get_q("p", C1, []) of
-            [] ->
+    Loc = case z_context:get_q("p", C1, "") of
+            "" ->
                 case z_context:get_req_header("referer", C1) of
                     undefined -> "/";
-                    Referrer ->  z_html:noscript(Referrer)
+                    Referrer ->  Referrer
                 end;
-            [$/|_] = Page ->
-                z_html:noscript(Page);
             Page ->
-                z_html:noscript([$/|Page])
+                Page
          end,
-    ?WM_REPLY({true, z_context:abs_url(Loc, C1)}, C1).
-
-
+    ?WM_REPLY({true, z_context:site_url(Loc, C1)}, C1).
 

modules/mod_translation/controllers/controller_language_set.erl

@@ -51,11 +51,12 @@ moved_temporarily(ReqData, Context0) ->
     Context = ?WM_REQ(ReqData, Context0),
     Context1 = mod_translation:set_user_language(z_context:get_q("code", Context), Context),
     Page = z_context:get_q("p", Context1),
-    Location = case z_utils:is_empty(Page) of
-                   true -> "/";
-                   false -> Page
-               end,
-    AbsUrl = z_context:abs_url(add_language(mod_translation:url_strip_language(Location), Context1), Context1),
+    Location = case Page of
+        "/" ++ _ -> Page;
+        _ -> "/"
+    end,
+    Location1 = z_sanitize:uri(Location),
+    AbsUrl = z_context:abs_url(add_language(mod_translation:url_strip_language(Location1), Context1), Context1),
     ?WM_REPLY({true, AbsUrl}, Context1).
 
 moved_permanently(ReqData, Context) ->

src/support/z_context.erl

@@ -46,6 +46,7 @@
     output/2,
 
     abs_url/2,
+    site_url/2,
 
     pickle/1,
     depickle/1,
@@ -400,7 +401,7 @@ prune_reqdata(ReqData) ->
 %% @spec abs_url(iolist(), Context) -> binary()
 abs_url(<<"http:", _/binary>> = Url, _Context) ->
     Url;
-abs_url(<<"https", _/binary>> = Url, _Context) ->
+abs_url(<<"https:", _/binary>> = Url, _Context) ->
     Url;
 abs_url(<<"//", _/binary>> = Url, Context) ->
     <<(site_protocol(Context))/binary, $:, Url/binary>>;
@@ -428,6 +429,41 @@ abs_url(Url, Context) ->
     has_url_protocol(_) ->
         false.
 
+%% @doc Ensure that the URL is for this host and that is sanitized.
+-spec site_url(undefined|string()|binary(), z:context()) -> binary().
+site_url(None, Context) when
+    None =:= undefined;
+    None =:= "";
+    None =:= <<>> ->
+    z_context:abs_url(<<"/">>, Context);
+site_url("#" ++ _ = Frag, _Context) ->
+    z_sanitize:uri(z_convert:to_binary(Frag));
+site_url(<<"#", _/binary>> = Frag, _Context) ->
+    z_sanitize:uri(Frag);
+site_url("/" ++ _ = Path, Context) ->
+    abs_url(Path, Context);
+site_url(<<"/", _/binary>> = Path, Context) ->
+    abs_url(Path, Context);
+site_url(Url, Context) ->
+    case z_convert:to_list(z_sanitize:uri(Url)) of
+        "#" ++ _ = Url1 ->
+            z_convert:to_binary(Url1);
+        "/" ++ _ = Url1 ->
+            abs_url(Url1, Context);
+        Url1 ->
+            Path1 = case mochiweb_util:urlsplit(Url1) of
+                {_Scheme, _Netloc, Path, "", ""} ->
+                    Path;
+                {_Scheme, _Netloc, Path, Query, ""} ->
+                    Path ++ "?" ++ Query;
+                {_Scheme, _Netloc, Path, "", Frag} ->
+                    Path ++ "#" ++ Frag;
+                {_Scheme, _Netloc, Path, Query, Frag} ->
+                    Path ++ "?" ++ Query ++ "#" ++ Frag
+            end,
+            abs_url(Path1, Context)
+    end.
+
 
 %% @doc Fetch the pid of the database worker pool for this site
 db_pool(#context{db={Pool, _Driver}}) ->