Check if response headers and values are acceptable (min OTP 22) (#26)

* Check if response headers and values are acceptable.

* Upgrade zotonic_stdlib, set min otp to 22

* Accept safe data urls for location response header.

.github/workflows/test.yml

@@ -18,7 +18,7 @@ jobs:
 
     strategy:
       matrix:
-        otp_version: [21.3,22,23]
+        otp_version: [22,23]
         os: [ubuntu-latest]
 
     container:

rebar.config

@@ -1,11 +1,11 @@
 %%-*- mode: erlang -*-
 
-{require_min_otp_vsn, "21.3"}.
+{require_min_otp_vsn, "22"}.
 
 {plugins, [rebar3_hex]}.
 
 {deps, [
-    {zotonic_stdlib, "1.2.8"},
+    {zotonic_stdlib, "1.2.10"},
     {cowboy, "2.8.0"}
 ]}.
 

rebar.lock

@@ -2,16 +2,16 @@
 [{<<"cowboy">>,{pkg,<<"cowboy">>,<<"2.8.0">>},0},
  {<<"cowlib">>,{pkg,<<"cowlib">>,<<"2.9.1">>},1},
  {<<"ranch">>,{pkg,<<"ranch">>,<<"1.7.1">>},1},
- {<<"zotonic_stdlib">>,{pkg,<<"zotonic_stdlib">>,<<"1.2.8">>},0}]}.
+ {<<"zotonic_stdlib">>,{pkg,<<"zotonic_stdlib">>,<<"1.2.10">>},0}]}.
 [
 {pkg_hash,[
  {<<"cowboy">>, <<"F3DC62E35797ECD9AC1B50DB74611193C29815401E53BAC9A5C0577BD7BC667D">>},
  {<<"cowlib">>, <<"61A6C7C50CF07FDD24B2F45B89500BB93B6686579B069A89F88CB211E1125C78">>},
  {<<"ranch">>, <<"6B1FAB51B49196860B733A49C07604465A47BDB78AA10C1C16A3D199F7F8C881">>},
- {<<"zotonic_stdlib">>, <<"AAF8C657B7D8C817F3014C0E80B05EEF685C0E77B63F5EDBE800454A2A1081A8">>}]},
+ {<<"zotonic_stdlib">>, <<"52849704922F9B1104E1925781868AD99640AB59C6D54F38F03D06B67F6332FD">>}]},
 {pkg_hash_ext,[
  {<<"cowboy">>, <<"4643E4FBA74AC96D4D152C75803DE6FAD0B3FA5DF354C71AFDD6CBEEB15FAC8A">>},
  {<<"cowlib">>, <<"E4175DC240A70D996156160891E1C62238EDE1729E45740BDD38064DAD476170">>},
  {<<"ranch">>, <<"451D8527787DF716D99DC36162FCA05934915DB0B6141BBDAC2EA8D3C7AFC7D7">>},
- {<<"zotonic_stdlib">>, <<"9F3D03AB7C998357656697A4A1A30D528E12B80C1AD4DB072541722A4BE3DD52">>}]}
+ {<<"zotonic_stdlib">>, <<"B41A3F2BE42854973C2D1198AFCA3F4C6426C3C4D04DF31B9B41DBFF572A0BBB">>}]}
 ].

src/cowmachine_req.erl

@@ -417,9 +417,27 @@ is_range_ok(Context) ->
 set_resp_header(Header, [V|Rs], Context) when is_binary(Header), is_binary(V) ->
     V1 = iolist_to_binary([V, [ [", ", R] || R <- Rs] ]),
     set_resp_header(Header, V1, Context);
+set_resp_header(Header, Value, Context)
+    when Header =:= <<"location">>;
+         Header =:= <<"content-location">> ->
+    case cowmachine_util:valid_location(Value) of
+        {true, Loc} ->
+            set_resp_header_1(Header, Loc, Context);
+        false ->
+            throw({http_invalid_location, Header, Value})
+    end;
 set_resp_header(Header, Value, Context) when is_binary(Header) ->
-    Req = cowboy_req:set_resp_header(Header, z_convert:to_binary(Value), req(Context)),
-    set_req(Req, Context).
+    set_resp_header_1(Header, Value, Context).
+
+set_resp_header_1(Header, Value, Context) ->
+    V = z_convert:to_binary(Value),
+    case cowmachine_util:is_valid_header(Header) andalso cowmachine_util:is_valid_header_value(V) of
+        true ->
+            Req = cowboy_req:set_resp_header(Header, z_convert:to_binary(Value), req(Context)),
+            set_req(Req, Context);
+        false ->
+            throw({http_invalid_header, Header, V})
+    end.
 
 %% @doc Set multiple response headers.
 -spec set_resp_headers([{binary(), binary()}], context()) -> context().

src/cowmachine_util.erl

@@ -18,6 +18,7 @@
 
 -module(cowmachine_util).
 
+-export([is_valid_header/1, is_valid_header_value/1, valid_location/1]).
 -export([parse_qs/1]).
 -export([convert_request_date/1]).
 -export([choose_media_type_provided/2]).
@@ -31,6 +32,43 @@
 -export([quoted_string/1]).
 -export([split_quoted_strings/1]).
 
+
+%% @doc Check header valid characters, see https://www.ietf.org/rfc/rfc822.txt
+-spec is_valid_header( binary() ) -> boolean().
+is_valid_header(<<>>) -> false;
+is_valid_header(H) -> is_valid_header_1(H).
+
+is_valid_header_1(<<>>) -> true;
+is_valid_header_1(<<$:, _/binary>>) -> false;
+is_valid_header_1(<<C, _/binary>>) when C >= $A, C =< $Z -> false;
+is_valid_header_1(<<C, R/binary>>) when C >= 33, C =< 126 -> is_valid_header_1(R);
+is_valid_header_1(_) -> false.
+
+%% @doc Check if the given value is acceptaple for a http header value.
+-spec is_valid_header_value( binary() ) -> boolean().
+is_valid_header_value(<<>>) -> true;
+is_valid_header_value(<<13, _/binary>>) -> false;
+is_valid_header_value(<<10, _/binary>>) -> false;
+is_valid_header_value(<<C, R/binary>>) when C =< 127 -> is_valid_header_value(R);
+is_valid_header_value(_) -> false.
+
+
+%% @doc Check if the given location is safe to use as a location header. This is
+%% uses as a defense against urls with scripts. The test is quite strict and will
+%% drop values that might have been acceptable.
+-spec valid_location( binary() ) -> {true, binary()} | false.
+valid_location(Location) ->
+    case is_valid_header_value(Location) of
+        true ->
+            case z_html:noscript(Location, false) of
+                <<"#script-removed">> -> false;
+                <<>> -> false;
+                Loc -> {true, Loc}
+            end;
+        false ->
+            false
+    end.
+
 %% @doc Parse the HTTP date (IMF-fixdate, rfc850, asctime).
 -spec convert_request_date(binary()) -> calendar:datetime().
 convert_request_date(Date) ->

test/cowmachine_security_tests.erl

@@ -0,0 +1,43 @@
+%% @author Marc Worrell <[email protected]>
+%% @hidden
+
+-module(cowmachine_security_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+cowmachine_metadata_test() ->
+    Req = #{
+        peer => {{127,0,0,1},1234},
+        scheme => <<"http">>,
+        port => 8000,
+        host => <<"local.dev">>,
+        headers => #{},
+        resp_headers => #{}
+    },
+    Context = cowmachine_req:init_context(Req, #{}, undefined),
+    _ = cowmachine_req:set_resp_header(<<"sample-header">>, <<"foobar">>, Context),
+    ok = try
+        cowmachine_req:set_resp_header(<<"sample-header">>, <<"foo", 13, 10, "bar">>, Context)
+    catch
+        throw:{http_invalid_header, _, _} -> ok
+    end,
+    ok = try
+        cowmachine_req:set_resp_header(<<"Foo-Bar">>, <<"foobar">>, Context)
+    catch
+        throw:{http_invalid_header, _, _} -> ok
+    end,
+    ok = try
+        cowmachine_req:set_resp_header(<<"Foo Bar">>, <<"foobar">>, Context)
+    catch
+        throw:{http_invalid_header, _, _} -> ok
+    end,
+    ok = try
+        cowmachine_req:set_resp_header(<<"Foo", 13, 10, "Bar">>, <<"foobar">>, Context)
+    catch
+        throw:{http_invalid_header, _, _} -> ok
+    end,
+    ok = try
+        cowmachine_req:set_resp_header(<<"location">>, <<"javascript:foobar">>, Context)
+    catch
+        throw:{http_invalid_location, _, _} -> ok
+    end.