Merge branch 'bmk/inets/httpd/prevent_xss_in_error_pages/OTP-9124' into bmk/inets/inet56_integration

Conflicts:
	lib/inets/doc/src/notes.xml
	lib/inets/src/inets_app/inets.appup.src

5 files changed, 97 insertions, 33 deletions

diff --git a/lib/inets/doc/src/notes.xml b/lib/inets/doc/src/notes.xml
index 5343d28976..05b5f6b6e8 100644
--- a/lib/inets/doc/src/notes.xml
+++ b/lib/inets/doc/src/notes.xml
@@ -52,6 +52,7 @@
         <item>
           <p>[ftp] Added (type) spec for all exported functions.</p>
           <p>Own Id: OTP-9114 Aux Id: seq11799</p>
+        </item>
 
         <item>
           <p>[httpd] 
@@ -60,7 +61,16 @@
           <p>Bernard Duggan</p>
           <p>Own Id: OTP-9123</p>
         </item>
+
+        <item>
+          <p>[httpd] Prevent XSS in error pages.
+	  Prevent user controlled input from being interpreted 
+	  as HTML in error pages by encoding the reserved HTML 
+	  characters. </p>
+	  <p>Michael Santos</p>
+          <p>Own Id: OTP-9124</p>
         </item>
+
       </list>
     </section>
 
diff --git a/lib/inets/src/http_lib/http_util.erl b/lib/inets/src/http_lib/http_util.erl
index 4f1147176c..5e6b69ac5e 100644
--- a/lib/inets/src/http_lib/http_util.erl
+++ b/lib/inets/src/http_lib/http_util.erl
@@ -25,7 +25,8 @@
 	 hexlist_to_integer/1, integer_to_hexlist/1, 
 	 convert_month/1, 
 	 is_hostname/1,
-	 timestamp/0, timeout/2
+	 timestamp/0, timeout/2,
+	 html_encode/1
 	]).
 
 
@@ -187,6 +188,13 @@ timeout(Timeout, Started) ->
     end.
     
 
+html_encode(Chars) ->
+    Reserved = sets:from_list([$&, $<, $>, $\", $', $/]),
+    lists:append(lists:map(fun(Char) ->
+	               char_to_html_entity(Char, Reserved)
+	           end, Chars)).
+
+
 %%%========================================================================
 %%% Internal functions
 %%%========================================================================
@@ -235,3 +243,11 @@ convert_to_ascii([Num | Reversed], Number)
 convert_to_ascii([Num | Reversed], Number) 
   when (Num > 9) andalso (Num < 16) ->
     convert_to_ascii(Reversed, [Num + 55 | Number]).
+
+char_to_html_entity(Char, Reserved) ->
+    case sets:is_element(Char, Reserved) of
+	true ->
+	    "&#" ++ integer_to_list(Char) ++ ";";
+	false ->
+	    [Char]
+    end.
diff --git a/lib/inets/src/http_server/httpd_util.erl b/lib/inets/src/http_server/httpd_util.erl
index 789f12652b..c1aff65d5e 100644
--- a/lib/inets/src/http_server/httpd_util.erl
+++ b/lib/inets/src/http_server/httpd_util.erl
@@ -181,7 +181,7 @@ message(304, _URL,_) ->
 message(400,none,_) ->
     "Your browser sent a query that this server could not understand.";
 message(400,Msg,_) ->
-    "Your browser sent a query that this server could not understand. "++ maybe_encode(Msg);
+    "Your browser sent a query that this server could not understand. "++ http_util:html_encode(Msg);
 message(401,none,_) ->
     "This server could not verify that you
 are authorized to access the document you
@@ -190,48 +190,48 @@ credentials (e.g., bad password), or your
 browser doesn't understand how to supply
 the credentials required.";
 message(403,RequestURI,_) ->
-    "You don't have permission to access "++ maybe_encode(RequestURI) ++" on this server.";
+    "You don't have permission to access "++ http_util:html_encode(RequestURI) ++" on this server.";
 message(404,RequestURI,_) ->
-    "The requested URL " ++ maybe_encode(RequestURI) ++ " was not found on this server.";
+    "The requested URL " ++ http_util:html_encode(RequestURI) ++ " was not found on this server.";
 message(408, Timeout, _) ->
     Timeout;
 message(412,none,_) ->
-    "The requested preconditions where false";
+    "The requested preconditions were false";
 message(413, Reason,_) ->
-    "Entity: " ++ Reason;
+    "Entity: " ++ http_util:html_encode(Reason);
 message(414,ReasonPhrase,_) ->
-    "Message "++ ReasonPhrase ++".";
+    "Message "++ http_util:html_encode(ReasonPhrase) ++".";
 message(416,ReasonPhrase,_) ->
-    ReasonPhrase;
+    http_util:html_encode(ReasonPhrase);
 
 message(500,_,ConfigDB) ->
     ServerAdmin=lookup(ConfigDB,server_admin,"unknown@unknown"),
     "The server encountered an internal error or "
 	"misconfiguration and was unable to complete "
 	"your request.<P>Please contact the server administrator "
-	++ ServerAdmin ++ ", and inform them of the time the error occurred "
+	++ http_util:html_encode(ServerAdmin) ++ ", and inform them of the time the error occurred "
 	"and anything you might have done that may have caused the error.";
 
 message(501,{Method, RequestURI, HTTPVersion}, _ConfigDB) ->
     if
 	is_atom(Method) ->
-	    atom_to_list(Method)++
-		" to "++ maybe_encode(RequestURI)++" ("++HTTPVersion++") not supported.";
+        http_util:html_encode(atom_to_list(Method))++
+		" to "++ http_util:html_encode(RequestURI)++" ("++ http_util:html_encode(HTTPVersion)++") not supported.";
 	is_list(Method) ->
-	    Method++
-		" to "++ maybe_encode(RequestURI)++" ("++HTTPVersion++") not supported."
+	    http_util:html_encode(Method)++
+		" to "++ http_util:html_encode(RequestURI)++" ("++ http_util:html_encode(HTTPVersion)++") not supported."
     end;
 
 message(503, String, _ConfigDB) ->
-    "This service in unavailable due to: "++String.
+    "This service in unavailable due to: "++ http_util:html_encode(String).
 
 maybe_encode(URI) ->
-    case lists:member($%, URI) of
-	true ->
-	    URI;
-	false ->
-	    http_uri:encode(URI)
-    end.
+    Decoded = try http_uri:decode(URI) of
+	N -> N
+    catch
+	error:_ -> URI
+    end,
+    http_uri:encode(Decoded).
 
 %%convert_rfc_date(Date)->{{YYYY,MM,DD},{HH,MIN,SEC}}
 
diff --git a/lib/inets/src/inets_app/inets.appup.src b/lib/inets/src/inets_app/inets.appup.src
index 02b1826c5f..cabc9c9834 100644
--- a/lib/inets/src/inets_app/inets.appup.src
+++ b/lib/inets/src/inets_app/inets.appup.src
@@ -21,6 +21,8 @@
   {"5.5.2", 
    [
     {load_module, ftp, soft_purge, soft_purge, []},
+    {load_module, http_util, soft_purge, soft_purge, []},
+    {load_module, httpd_util, soft_purge, soft_purge, [http_util]},
     {load_module, mod_esi, soft_purge, soft_purge, []},
     {load_module, httpc, soft_purge, soft_purge, [httpc_handler]},
     {load_module, httpc_request, soft_purge, soft_purge, []},
@@ -31,6 +33,8 @@
    [
     {load_module, ftp, soft_purge, soft_purge, []}, 
     {load_module, http_chunk, soft_purge, soft_purge, []},
+    {load_module, http_util, soft_purge, soft_purge, []},
+    {load_module, httpd_util, soft_purge, soft_purge, [http_util]},
     {load_module, mod_esi, soft_purge, soft_purge, []},
     {load_module, httpc, soft_purge, soft_purge, [httpc_handler]},
     {load_module, httpc_request, soft_purge, soft_purge, []},
@@ -53,6 +57,8 @@
   {"5.5.2",
    [
     {load_module, ftp, soft_purge, soft_purge, []}, 
+    {load_module, http_util, soft_purge, soft_purge, []},
+    {load_module, httpd_util, soft_purge, soft_purge, [http_util]},
     {load_module, mod_esi, soft_purge, soft_purge, []},
     {load_module, httpc, soft_purge, soft_purge, [httpc_handler]},
     {load_module, httpc_request, soft_purge, soft_purge, []},
@@ -63,6 +69,8 @@
    [
     {load_module, ftp, soft_purge, soft_purge, []}, 
     {load_module, http_chunk, soft_purge, soft_purge, []},
+    {load_module, http_util, soft_purge, soft_purge, []},
+    {load_module, httpd_util, soft_purge, soft_purge, [http_util]},
     {load_module, mod_esi, soft_purge, soft_purge, []},
     {load_module, httpc, soft_purge, soft_purge, [httpc_handler]},
     {load_module, httpc_request, soft_purge, soft_purge, []},
diff --git a/lib/inets/test/httpd_basic_SUITE.erl b/lib/inets/test/httpd_basic_SUITE.erl
index 3e29b68283..f23d0b4765 100644
--- a/lib/inets/test/httpd_basic_SUITE.erl
+++ b/lib/inets/test/httpd_basic_SUITE.erl
@@ -29,7 +29,11 @@
 suite() -> [{ct_hooks,[ts_install_cth]}].
 
 all() -> 
-    [uri_too_long_414, header_too_long_413, escaped_url_in_error_body].
+    [
+     uri_too_long_414, 
+     header_too_long_413, 
+     escaped_url_in_error_body
+    ].
 
 groups() -> 
     [].
@@ -40,6 +44,7 @@ init_per_group(_GroupName, Config) ->
 end_per_group(_GroupName, Config) ->
     Config.
 
+
 %%--------------------------------------------------------------------
 %% Function: init_per_suite(Config) -> Config
 %% Config - [tuple()]
@@ -50,6 +55,8 @@ end_per_group(_GroupName, Config) ->
 %% variable, but should NOT alter/remove any existing entries.
 %%--------------------------------------------------------------------
 init_per_suite(Config) ->
+    tsp("init_per_suite -> entry with"
+	"~n   Config: ~p", [Config]),
     ok = inets:start(),
     PrivDir = ?config(priv_dir, Config),
     HttpdConf = [{port, 0}, {ipfamily, inet}, 
@@ -64,6 +71,8 @@ init_per_suite(Config) ->
 %% Description: Cleanup after the whole suite
 %%--------------------------------------------------------------------
 end_per_suite(_Config) ->
+    tsp("end_per_suite -> entry with"
+	"~n   Config: ~p", [_Config]),
     inets:stop(),
     ok.
 
@@ -79,9 +88,12 @@ end_per_suite(_Config) ->
 %% Note: This function is free to add any key/value pairs to the Config
 %% variable, but should NOT alter/remove any existing entries.
 %%--------------------------------------------------------------------
-init_per_testcase(_Case, Config) ->
+init_per_testcase(Case, Config) ->
+    tsp("init_per_testcase(~w) -> entry with"
+	"~n   Config: ~p", [Case, Config]),
     Config.
 
+
 %%--------------------------------------------------------------------
 %% Function: end_per_testcase(Case, Config) -> _
 %% Case - atom()
@@ -90,9 +102,12 @@ init_per_testcase(_Case, Config) ->
 %%   A list of key/value pairs, holding the test case configuration.
 %% Description: Cleanup after each test case
 %%--------------------------------------------------------------------
-end_per_testcase(_, Config) ->
+end_per_testcase(Case, Config) ->
+    tsp("end_per_testcase(~w) -> entry with"
+	"~n   Config: ~p", [Case, Config]),
     Config.
 
+
 %%-------------------------------------------------------------------------
 %% Test cases starts here.
 %%-------------------------------------------------------------------------
@@ -142,22 +157,30 @@ escaped_url_in_error_body(doc) ->
 escaped_url_in_error_body(suite) ->
     [];
 escaped_url_in_error_body(Config) when is_list(Config) ->
-    HttpdConf =   ?config(httpd_conf, Config),
+    tsp("escaped_url_in_error_body -> entry with"
+	"~n   Config: ~p", [Config]),
+    HttpdConf = ?config(httpd_conf, Config),
     {ok, Pid} = inets:start(httpd, [{port, 0} | HttpdConf]),
     Info = httpd:info(Pid),
     Port = proplists:get_value(port, Info),
-    Address = proplists:get_value(bind_address, Info),
-    Path = "/<b>this_is_bold<b>",
+    _Address = proplists:get_value(bind_address, Info),
+    Path = "/<b>this_is_bold</b>",
     URL = ?URL_START ++ integer_to_list(Port) ++ Path,
     EscapedPath = http_uri:encode(Path),
-    {ok, {404, Body}} = httpc:request(get, {URL, []},
-				      [{url_encode, true}],
-				      [{version, "HTTP/1.0"}, {full_result, false}]),
-    EscapedPath = find_URL_path(string:tokens(Body, " ")),
-    {ok, {404, Body1}} = httpc:request(get, {URL, []}, [],
-				       [{version, "HTTP/1.0"}, {full_result, false}]),
+    {ok, {404, Body1}} = httpc:request(get, {URL, []},
+				       [{url_encode, true}, 
+					{version,    "HTTP/1.0"}],
+				      [{full_result, false}]),
     EscapedPath = find_URL_path(string:tokens(Body1, " ")),
-    inets:stop(httpd, Pid).
+    {ok, {404, Body2}} = httpc:request(get, {URL, []},
+				       [{url_encode,  false}, 
+					{version,     "HTTP/1.0"}], 
+				       [{full_result, false}]),
+    HTMLEncodedPath = http_util:html_encode(Path),
+    HTMLEncodedPath = find_URL_path(string:tokens(Body2, " ")),
+    inets:stop(httpd, Pid),
+    tsp("escaped_url_in_error_body -> done"),
+    ok.
 
 find_URL_path([]) ->
     "";
@@ -165,3 +188,10 @@ find_URL_path(["URL", URL | _]) ->
     URL;
 find_URL_path([_ | Rest]) ->
     find_URL_path(Rest).
+
+
+tsp(F) ->
+    tsp(F, []).
+tsp(F, A) ->
+    test_server:format("~p ~p:" ++ F ++ "~n", [self(), ?MODULE | A]).
+