From f9060599aeab81cb9282ddf51cc057bf1353208f Mon Sep 17 00:00:00 2001
From: Micael Karlberg <bmk@erlang.org>
Date: Tue, 25 Oct 2011 12:34:56 +0200
Subject: The XSS prevention methods used was confused if the URL was encoded
 (hex-encoded). OTP-9655

---
 lib/inets/src/http_lib/http_util.erl     |   5 +-
 lib/inets/src/http_server/httpd_file.erl |   4 +-
 lib/inets/src/http_server/httpd_util.erl |  25 +++---
 lib/inets/test/httpc_SUITE.erl           |   3 +-
 lib/inets/test/httpd_basic_SUITE.erl     | 144 +++++++++++++++++++++++++------
 5 files changed, 137 insertions(+), 44 deletions(-)

diff --git a/lib/inets/src/http_lib/http_util.erl b/lib/inets/src/http_lib/http_util.erl
index be0602ff6e..5d8cb9365d 100644
--- a/lib/inets/src/http_lib/http_util.erl
+++ b/lib/inets/src/http_lib/http_util.erl
@@ -190,9 +190,8 @@ timeout(Timeout, Started) ->
     
 html_encode(Chars) ->
     Reserved = sets:from_list([$&, $<, $>, $\", $', $/]),
-    lists:append(lists:map(fun(Char) ->
-				   char_to_html_entity(Char, Reserved)
-			   end, Chars)).
+    lists:append([char_to_html_entity(Char, Reserved) || Char <- Chars]).
+
 
 
 %%%========================================================================
diff --git a/lib/inets/src/http_server/httpd_file.erl b/lib/inets/src/http_server/httpd_file.erl
index fbe713ecd1..4490a6356a 100644
--- a/lib/inets/src/http_server/httpd_file.erl
+++ b/lib/inets/src/http_server/httpd_file.erl
@@ -39,8 +39,8 @@ handle_error(_Reason, Op, _ModData, Path) ->
     handle_error(500, Op, none, Path, "").
 	    
 handle_error(StatusCode, Op, none, Path, Reason) ->
-    {StatusCode, none, ?NICE("Can't " ++ Op ++ Path ++ Reason)};
+    {StatusCode, none, ?NICE("Can't " ++ Op ++ " " ++ Path ++ Reason)};
 
 handle_error(StatusCode, Op, ModData, Path, Reason) ->
     {StatusCode, ModData#mod.request_uri,
-     ?NICE("Can't " ++ Op ++ Path ++ Reason)}.
+     ?NICE("Can't " ++ Op ++ " " ++ Path ++ Reason)}.
diff --git a/lib/inets/src/http_server/httpd_util.erl b/lib/inets/src/http_server/httpd_util.erl
index 7fe5d6d152..2e0752bcc0 100644
--- a/lib/inets/src/http_server/httpd_util.erl
+++ b/lib/inets/src/http_server/httpd_util.erl
@@ -180,10 +180,10 @@ message(301,URL,_) ->
 message(304, _URL,_) ->
     "The document has not been changed.";
 message(400, none, _) ->
-    "Your browser sent a query that this server could not understand.";
+    "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. " ++ 
-	http_util:html_encode(Msg);
+	html_encode(http_uri:decode(Msg));
 message(401, none, _) ->
     "This server could not verify that you
 are authorized to access the document you
@@ -193,29 +193,29 @@ browser doesn't understand how to supply
 the credentials required.";
 message(403,RequestURI,_) ->
     "You don't have permission to access " ++ 
-	http_util:html_encode(RequestURI) ++ 
+	html_encode(RequestURI) ++ 
 	" on this server.";
 message(404,RequestURI,_) ->
     "The requested URL " ++ 
-	http_util:html_encode(RequestURI) ++ 
+	html_encode(RequestURI) ++ 
 	" was not found on this server.";
 message(408, Timeout, _) ->
     Timeout;
 message(412,none,_) ->
     "The requested preconditions where false";
 message(413, Reason,_) ->
-    "Entity: " ++ http_util:html_encode(Reason);
+    "Entity: " ++ html_encode(Reason);
 message(414,ReasonPhrase,_) ->
-    "Message " ++ http_util:html_encode(ReasonPhrase) ++ ".";
+    "Message " ++ html_encode(ReasonPhrase) ++ ".";
 message(416,ReasonPhrase,_) ->
-    http_util:html_encode(ReasonPhrase);
+    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 "
-	++ http_util:html_encode(ServerAdmin) ++ 
+	++ html_encode(ServerAdmin) ++ 
 	", and inform them of the time the error occurred "
 	"and anything you might have done that may have caused the error.";
 
@@ -224,17 +224,17 @@ message(501,{Method, RequestURI, HTTPVersion}, _ConfigDB) ->
 	is_atom(Method) ->
 	    atom_to_list(Method)++
 		" to " ++ 
-		http_util:html_encode(RequestURI) ++ 
+		html_encode(RequestURI) ++ 
 		" (" ++ HTTPVersion ++ ") not supported.";
 	is_list(Method) ->
 	    Method++
 		" to " ++ 
-		http_util:html_encode(RequestURI) ++ 
+		html_encode(RequestURI) ++ 
 		" (" ++ HTTPVersion ++ ") not supported."
     end;
 
 message(503, String, _ConfigDB) ->
-    "This service in unavailable due to: " ++ http_util:html_encode(String).
+    "This service in unavailable due to: " ++ html_encode(String).
 
 maybe_encode(URI) ->
     case lists:member($%, URI) of
@@ -244,6 +244,9 @@ maybe_encode(URI) ->
 			     http_uri:encode(URI)
 		     end.
 
+html_encode(String) ->
+    http_util:html_encode(http_uri:decode(String)).
+
 %%convert_rfc_date(Date)->{{YYYY,MM,DD},{HH,MIN,SEC}}
 
 convert_request_date([D,A,Y,DateType| Rest])->
diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl
index 71f017dae6..c752b89aa0 100644
--- a/lib/inets/test/httpc_SUITE.erl
+++ b/lib/inets/test/httpc_SUITE.erl
@@ -3112,7 +3112,8 @@ tsp(F) ->
     tsp(F, []).
 tsp(F, A) ->
     Timestamp = formated_timestamp(), 
-    test_server:format("** ~s ** ~p ~p:" ++ F ++ "~n", [Timestamp, self(), ?MODULE | A]).
+    test_server:format("** ~s ** ~p ~p:" ++ F ++ "~n", 
+		       [Timestamp, self(), ?MODULE | A]).
 
 formated_timestamp() ->
     format_timestamp( os:timestamp() ).
diff --git a/lib/inets/test/httpd_basic_SUITE.erl b/lib/inets/test/httpd_basic_SUITE.erl
index ed0fe942cf..581c9c5782 100644
--- a/lib/inets/test/httpd_basic_SUITE.erl
+++ b/lib/inets/test/httpd_basic_SUITE.erl
@@ -49,9 +49,28 @@ all(suite) ->
 init_per_suite(Config) ->
     ok = inets:start(),
     PrivDir = ?config(priv_dir, Config),
-    HttpdConf = [{port, 0}, {ipfamily, inet}, 
-		 {server_name, "httpd_test"}, {server_root, PrivDir},
-		 {document_root, PrivDir}, {bind_address, "localhost"}],
+
+    Dummy = 
+"<HTML>
+<HEAD>
+<TITLE>/index.html</TITLE>
+</HEAD>
+<BODY>
+DUMMY
+</BODY>
+</HTML>",
+
+    DummyFile = filename:join([PrivDir,"dummy.html"]),
+    {ok, Fd}  = file:open(DummyFile, [write]),
+    ok        = file:write(Fd, Dummy),
+    ok        = file:close(Fd), 
+    HttpdConf = [{port,          0}, 
+		 {ipfamily,      inet}, 
+		 {server_name,   "httpd_test"}, 
+		 {server_root,   PrivDir},
+		 {document_root, PrivDir}, 
+		 {bind_address,  "localhost"}],
+
     [{httpd_conf, HttpdConf} |  Config].
 
 %%--------------------------------------------------------------------
@@ -115,6 +134,10 @@ uri_too_long_414(Config) when is_list(Config) ->
  				        {version, "HTTP/0.9"}]),    
     inets:stop(httpd, Pid).
     
+
+%%-------------------------------------------------------------------------
+%%-------------------------------------------------------------------------
+
 header_too_long_413(doc) ->
     ["Test that too long headers's get 413 HTTP code"];
 header_too_long_413(suite) ->
@@ -135,49 +158,92 @@ header_too_long_413(Config) when is_list(Config) ->
     inets:stop(httpd, Pid).
    
 
+%%-------------------------------------------------------------------------
+%%-------------------------------------------------------------------------
+
 escaped_url_in_error_body(doc) ->
     ["Test Url-encoding see OTP-8940"];
 escaped_url_in_error_body(suite) ->
     [];
 escaped_url_in_error_body(Config) when is_list(Config) ->
+    tsp("escaped_url_in_error_body -> entry"),
     HttpdConf   = ?config(httpd_conf, Config),
     {ok, Pid}   = inets:start(httpd, [{port, 0} | HttpdConf]),
     Info        = httpd:info(Pid),
-    Port        = proplists:get_value(port, Info),
+    Port        = proplists:get_value(port,         Info),
     _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),
-    case httpc:request(get, {URL, []},
-		       [{url_encode, true}, 
-			{version, "HTTP/1.0"}],
+
+    %% Request 1
+    tsp("escaped_url_in_error_body -> request 1"),
+    URL1        = ?URL_START ++ integer_to_list(Port),
+    %% Make sure the server is ok, by making a request for a valid page
+    case httpc:request(get, {URL1 ++ "/dummy.html", []},
+		       [{url_encode,  false}, 
+			{version,     "HTTP/1.0"}],
 		       [{full_result, false}]) of
-	{ok, {404, Body1}} ->
-	    case find_URL_path(string:tokens(Body1, " ")) of
-		EscapedPath ->
-		    ok;
-		BadPath1 ->
-		    tsf({unexpected_path_1, EscapedPath, BadPath1})
-	    end;
+	{ok, {200, _}} ->
+	    %% Don't care about the the body, just that we get a ok response
+	    ok;
 	{ok, UnexpectedOK1} ->
 	    tsf({unexpected_ok_1, UnexpectedOK1})
     end,
 
-    case httpc:request(get, {URL, []}, 
-		       [{version, "HTTP/1.0"}],
+    %% Request 2
+    tsp("escaped_url_in_error_body -> request 2"),
+    %% Make sure the server is ok, by making a request for a valid page
+    case httpc:request(get, {URL1 ++ "/dummy.html", []},
+		       [{url_encode,  true}, 
+			{version,     "HTTP/1.0"}],
+		       [{full_result, false}]) of
+	{ok, {200, _}} ->
+	    %% Don't care about the the body, just that we get a ok response
+	    ok;
+	{ok, UnexpectedOK2} ->
+	    tsf({unexpected_ok_2, UnexpectedOK2})
+    end,
+
+    %% Request 3
+    tsp("escaped_url_in_error_body -> request 3"),
+    %% Ask for a non-existing page(1)
+    Path            = "/<b>this_is_bold<b>",
+    HTMLEncodedPath = http_util:html_encode(Path),
+    URL2            = URL1 ++ Path,
+    case httpc:request(get, {URL2, []},
+		       [{url_encode,  true}, 
+			{version,     "HTTP/1.0"}],
 		       [{full_result, false}]) of
-	{ok, {404, Body2}} ->
-	    HTMLEncodedPath = http_util:html_encode(Path),
-	    case find_URL_path(string:tokens(Body2, " ")) of
+	{ok, {404, Body3}} ->
+	    case find_URL_path(string:tokens(Body3, " ")) of
 		HTMLEncodedPath ->
 		    ok;
-		BadPath2 ->
-		    tsf({unexpected_path_2, EscapedPath, BadPath2})
+		BadPath3 ->
+		    tsf({unexpected_path_3, HTMLEncodedPath, BadPath3})
 	    end;
-	{ok, UnexpectedOK2} ->
-	    tsf({unexpected_ok_2, UnexpectedOK2})
+	{ok, UnexpectedOK3} ->
+	    tsf({unexpected_ok_1, UnexpectedOK3})
+    end,
+
+    %% Request 4
+    tsp("escaped_url_in_error_body -> request 4"),
+    %% Ask for a non-existing page(2)
+    case httpc:request(get, {URL2, []}, 
+		       [{url_encode,  false}, 
+			{version,     "HTTP/1.0"}],
+		       [{full_result, false}]) of
+	{ok, {404, Body4}} ->
+	    case find_URL_path(string:tokens(Body4, " ")) of
+		HTMLEncodedPath ->
+		    ok;
+		BadPath4 ->
+		    tsf({unexpected_path_2, HTMLEncodedPath, BadPath4})
+	    end;
+	{ok, UnexpectedOK4} ->
+	    tsf({unexpected_ok_4, UnexpectedOK4})
     end, 
-    inets:stop(httpd, Pid).
+    tsp("escaped_url_in_error_body -> stop inets"),
+    inets:stop(httpd, Pid),
+    tsp("escaped_url_in_error_body -> done"),    
+    ok.
 
 find_URL_path([]) ->
     "";
@@ -189,3 +255,27 @@ find_URL_path([_ | Rest]) ->
 tsf(Reason) ->
     test_server:fail(Reason).
 
+
+tsp(F) ->
+    tsp(F, []).
+tsp(F, A) ->
+    Timestamp = formated_timestamp(), 
+    test_server:format("** ~s ** ~p ~p:" ++ F ++ "~n", 
+		       [Timestamp, self(), ?MODULE | A]).
+
+formated_timestamp() ->
+    format_timestamp( os:timestamp() ).
+
+format_timestamp({_N1, _N2, N3} = Now) ->
+    {Date, Time}   = calendar:now_to_datetime(Now),
+    {YYYY,MM,DD}   = Date,
+    {Hour,Min,Sec} = Time,
+    FormatDate =
+        io_lib:format("~.4w:~.2.0w:~.2.0w ~.2.0w:~.2.0w:~.2.0w 4~w",
+                      [YYYY,MM,DD,Hour,Min,Sec,round(N3/1000)]),
+    lists:flatten(FormatDate).
+
+
+skip(Reason) ->
+    {skip, Reason}.
+
-- 
cgit v1.2.3