[erlang-patches] [PATCH] inets: prevent XSS in error pages

[Niclas Axelsson]

On 02/21/2011 07:53 PM, Michael Santos wrote:
> 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([]) ->
>    
Thanks Michael,

Your patch is now included in 'pu'.

Regards,
Niclas Axelsson, Erlang/OTP