[PATCH] inets: prevent XSS in error pages

[Michael Santos]

Prevent user controlled input from being interpreted as HTML in error
pages by encoding the reserved HTML characters. The reserved character
set should be safe for displaying data within the body of HTML pages
as outlined here:

http://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

Previously, weird URLs were URI encoded in the error page. This worked
quite well but the URL would be displayed in the HTML in percent encoded
format. There was also a check for URIs that were already escaped (by
the browser) that would fail if the browser sent an URI containing a
"%", e.g.:

w3m "http://localhost:8080/<b>foo</b>?%"

Also encode the HTTP method and version, since it's possible they may be
manipulated:

<b>FOO</b> /index.html HTTP/1.0
GET /index.html <b>foo</b>/1.0

Encode the static messages to prevent characters from being interpreted
as HTML such as "heavy load (>~w processes)".
---
 lib/inets/src/http_lib/http_util.erl     |   18 +++++++++++++-
 lib/inets/src/http_server/httpd_util.erl |   38 +++++++++++++++---------------
 lib/inets/test/httpd_basic_SUITE.erl     |   11 ++++----
 3 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/lib/inets/src/http_lib/http_util.erl b/lib/inets/src/http_lib/http_util.erl
index 4f11471..5e6b69a 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 789f126..c1aff65 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@REDACTED"),
     "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/test/httpd_basic_SUITE.erl b/lib/inets/test/httpd_basic_SUITE.erl
index 9ba2e73..cdd3350 100644
--- a/lib/inets/test/httpd_basic_SUITE.erl
+++ b/lib/inets/test/httpd_basic_SUITE.erl
@@ -148,12 +148,13 @@ escaped_url_in_error_body(Config) when is_list(Config) ->
     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}]),
+				      [{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}]),
-    EscapedPath = find_URL_path(string:tokens(Body1, " ")),
+    {ok, {404, Body1}} = httpc:request(get, {URL, []},
+				      [{version, "HTTP/1.0"}], [{full_result, false}]),
+    HTMLEncodedPath = http_util:html_encode(Path),
+    HTMLEncodedPath = find_URL_path(string:tokens(Body1, " ")),
     inets:stop(httpd, Pid).
 
 find_URL_path([]) ->
-- 
1.7.0.4