From 4427108b69fcd1e6a8233a217fa0e99d0564b714 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Sat, 7 Sep 2019 12:18:16 +0200 Subject: Improve the cowboy_static consistency across platforms As a result we explictly reject path_info components that include a forward slash, backward slash or NUL character. This only applies to the [...] part of the path for dir/priv_dir configuration. Also improve the tests so that they work on Windows. --- src/cowboy_static.erl | 70 ++++++++++++++++++++++++++----------------- test/static_handler_SUITE.erl | 33 +++++++++++++------- 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/src/cowboy_static.erl b/src/cowboy_static.erl index d6ef45c..b0cf146 100644 --- a/src/cowboy_static.erl +++ b/src/cowboy_static.erl @@ -119,35 +119,51 @@ init_dir(Req, Path, HowToAccess, Extra) when is_list(Path) -> init_dir(Req, list_to_binary(Path), HowToAccess, Extra); init_dir(Req, Path, HowToAccess, Extra) -> Dir = fullpath(filename:absname(Path)), - PathInfo = cowboy_req:path_info(Req), - Filepath = filename:join([Dir|escape_reserved(PathInfo)]), - Len = byte_size(Dir), - case fullpath(Filepath) of - << Dir:Len/binary, $/, _/binary >> -> - init_info(Req, Filepath, HowToAccess, Extra); - << Dir:Len/binary >> -> - init_info(Req, Filepath, HowToAccess, Extra); - _ -> - {cowboy_rest, Req, error} + case cowboy_req:path_info(Req) of + %% When dir/priv_dir are used and there is no path_info + %% this is a configuration error and we abort immediately. + undefined -> + {ok, cowboy_req:reply(500, Req), error}; + PathInfo -> + case validate_reserved(PathInfo) of + error -> + {cowboy_rest, Req, error}; + ok -> + Filepath = filename:join([Dir|PathInfo]), + Len = byte_size(Dir), + case fullpath(Filepath) of + << Dir:Len/binary, $/, _/binary >> -> + init_info(Req, Filepath, HowToAccess, Extra); + << Dir:Len/binary >> -> + init_info(Req, Filepath, HowToAccess, Extra); + _ -> + {cowboy_rest, Req, error} + end + end end. -escape_reserved([]) -> []; -escape_reserved([P|Tail]) -> [escape_reserved(P, <<>>)|escape_reserved(Tail)]. +validate_reserved([]) -> + ok; +validate_reserved([P|Tail]) -> + case validate_reserved1(P) of + ok -> validate_reserved(Tail); + error -> error + end. -%% We escape the slash found in path segments because -%% a segment corresponds to a directory entry, and -%% therefore those slashes are expected to be part of -%% the directory name. -%% -%% Note that on most systems the slash is prohibited -%% and cannot appear in filenames, which means the -%% requested file will end up being not found. -escape_reserved(<<>>, Acc) -> - Acc; -escape_reserved(<< $/, Rest/bits >>, Acc) -> - escape_reserved(Rest, << Acc/binary, $\\, $/ >>); -escape_reserved(<< C, Rest/bits >>, Acc) -> - escape_reserved(Rest, << Acc/binary, C >>). +%% We always reject forward slash, backward slash and NUL as +%% those have special meanings across the supported platforms. +%% We could support the backward slash on some platforms but +%% for the sake of consistency and simplicity we don't. +validate_reserved1(<<>>) -> + ok; +validate_reserved1(<<$/, _/bits>>) -> + error; +validate_reserved1(<<$\\, _/bits>>) -> + error; +validate_reserved1(<<0, _/bits>>) -> + error; +validate_reserved1(<<_, Rest/bits>>) -> + validate_reserved1(Rest). fullpath(Path) -> fullpath(filename:split(Path), []). @@ -290,7 +306,7 @@ bad_path_win32_check_test_() -> -endif. %% Reject requests that tried to access a file outside -%% the target directory. +%% the target directory, or used reserved characters. -spec malformed_request(Req, State) -> {boolean(), Req, State}. diff --git a/test/static_handler_SUITE.erl b/test/static_handler_SUITE.erl index 7cee876..c06268e 100644 --- a/test/static_handler_SUITE.erl +++ b/test/static_handler_SUITE.erl @@ -67,13 +67,14 @@ init_per_suite(Config) -> true = code:add_pathz(filename:join( [config(data_dir, Config), "static_files_app", "ebin"])), ok = application:load(static_files_app), - %% A special folder contains files of 1 character from 0 to 127. + %% A special folder contains files of 1 character from 1 to 127 + %% excluding / and \ as they are always rejected. CharDir = config(priv_dir, Config) ++ "/char", ok = filelib:ensure_dir(CharDir ++ "/file"), Chars0 = lists:flatten([case file:write_file(CharDir ++ [$/, C], [C]) of ok -> C; {error, _} -> [] - end || C <- lists:seq(0, 127)]), + end || C <- (lists:seq(1, 127) -- "/\\")]), %% Determine whether we are on a case insensitive filesystem and %% remove uppercase characters in that case. On case insensitive %% filesystems we end up overwriting the "A" file with the "a" contents. @@ -134,7 +135,8 @@ init_large_file(Filename) -> "" = os:cmd("truncate -s 32M " ++ Filename), ok; {win32, _} -> - ok + Size = 32*1024*1024, + ok = file:write_file(Filename, <<0:Size/unit:8>>) end. %% Routes. @@ -458,21 +460,28 @@ dir_error_slash(Config) -> {403, _, _} = do_get(config(prefix, Config) ++ "//", Config), ok. -dir_error_slash_urlencoded(Config) -> - doc("Try to get a file named '/' percent encoded."), - {404, _, _} = do_get(config(prefix, Config) ++ "/%2f", Config), +dir_error_reserved_urlencoded(Config) -> + doc("Try to get a file named '/' or '\\' or 'NUL' percent encoded."), + {400, _, _} = do_get(config(prefix, Config) ++ "/%2f", Config), + {400, _, _} = do_get(config(prefix, Config) ++ "/%5c", Config), + {400, _, _} = do_get(config(prefix, Config) ++ "/%00", Config), ok. dir_error_slash_urlencoded_dotdot_file(Config) -> doc("Try to use a percent encoded slash to access an existing file."), {200, _, _} = do_get(config(prefix, Config) ++ "/directory/../style.css", Config), - {404, _, _} = do_get(config(prefix, Config) ++ "/directory%2f../style.css", Config), + {400, _, _} = do_get(config(prefix, Config) ++ "/directory%2f../style.css", Config), ok. dir_error_unreadable(Config) -> - doc("Try to get a file that can't be read."), - {403, _, _} = do_get(config(prefix, Config) ++ "/unreadable", Config), - ok. + case os:type() of + {win32, _} -> + {skip, "ACL not enabled by default under MSYS2."}; + {unix, _} -> + doc("Try to get a file that can't be read."), + {403, _, _} = do_get(config(prefix, Config) ++ "/unreadable", Config), + ok + end. dir_html(Config) -> doc("Get a .html file."), @@ -899,10 +908,12 @@ unicode_basic_latin(Config) -> "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "0123456789" ":@-_~!$&'()*+,;=", - Chars = case config(case_sensitive, Config) of + Chars1 = case config(case_sensitive, Config) of false -> Chars0 -- "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; true -> Chars0 end, + %% Remove the characters for which we have no corresponding file. + Chars = Chars1 -- (Chars1 -- config(chars, Config)), _ = [case do_get("/char/" ++ [C], Config) of {200, _, << C >>} -> ok; Error -> exit({error, C, Error}) -- cgit v1.2.3