diff --git a/rebar.config b/rebar.config index 337dbb3..72ca09a 100644 --- a/rebar.config +++ b/rebar.config @@ -1,7 +1,8 @@ %% -*- mode: erlang; flycheck-mode: nil -*- {erl_opts, - [debug_info]}. + [debug_info, + {platform_define, "^(18|(19\.[012]))", custom_safe_relative_path}]}. %% HACK {overrides, diff --git a/src/elli_static.erl b/src/elli_static.erl index cf38536..e4330b6 100644 --- a/src/elli_static.erl +++ b/src/elli_static.erl @@ -9,7 +9,7 @@ -include_lib("kernel/include/file.hrl"). -define(TABLE, elli_static_table). - +-define(NOT_FOUND, {404, [], <<"Not Found">>}). %% elli_handler callbacks -export([handle/2, handle_event/3]). @@ -81,15 +81,31 @@ file_info(Filename) -> {error, Reason} -> throw(Reason) end. - maybe_file(Req, Prefix, Dir) -> + %% all paths start with a slash which `safe_relative_path/1' interprets as + %% unsafe (absolute path), so temporarily remove it + <<"/", RawPath/binary>> = elli_request:raw_path(Req), + + %% santize the path ensuring the request doesn't access any parent + %% directories ... and reattach the slash if deemed safe + SafePath = case elli_static_utils:safe_relative_path(RawPath) of + unsafe -> + throw(?NOT_FOUND); + %% return type quirk work around + [] -> + <<"/">>; + Sanitized -> + <<"/", Sanitized/binary>> + end, + Size = byte_size(Prefix), - case elli_request:raw_path(Req) of + case SafePath of + %% ensure that `SafePath' starts with the correct prefix <> -> Filename = filename:join(Dir, Path), case filelib:is_regular(Filename) of true -> {just, Filename}; - false -> throw({404, [], <<"Not Found">>}) + false -> throw(?NOT_FOUND) end; _ -> nothing diff --git a/src/elli_static_utils.erl b/src/elli_static_utils.erl new file mode 100644 index 0000000..3136d5b --- /dev/null +++ b/src/elli_static_utils.erl @@ -0,0 +1,44 @@ +-module(elli_static_utils). + +-export([safe_relative_path/1]). + +%% @doc Backport of `filename:safe_relative_path/1' from 19.3. This code was +%% lifted from: +%% https://github.com/erlang/otp/blob/master/lib/stdlib/src/filename.erl#L811 +-spec safe_relative_path(Filename) -> 'unsafe' | SafeFilename when + Filename :: file:name_all(), + SafeFilename :: file:name_all(). + +-ifndef(custom_safe_relative_path). +safe_relative_path(Path) -> + filename:safe_relative_path(Path). +-else. +safe_relative_path(Path) -> + case filename:pathtype(Path) of + relative -> + Cs0 = filename:split(Path), + safe_relative_path_1(Cs0, []); + _ -> + unsafe + end. + +safe_relative_path_1(["."|T], Acc) -> + safe_relative_path_1(T, Acc); +safe_relative_path_1([<<".">>|T], Acc) -> + safe_relative_path_1(T, Acc); +safe_relative_path_1([".."|T], Acc) -> + climb(T, Acc); +safe_relative_path_1([<<"..">>|T], Acc) -> + climb(T, Acc); +safe_relative_path_1([H|T], Acc) -> + safe_relative_path_1(T, [H|Acc]); +safe_relative_path_1([], []) -> + []; +safe_relative_path_1([], Acc) -> + filename:join(lists:reverse(Acc)). + +climb(_, []) -> + unsafe; +climb(T, [_|Acc]) -> + safe_relative_path_1(T, Acc). +-endif. diff --git a/test/elli_static_tests.erl b/test/elli_static_tests.erl index b90adeb..a0b14d7 100644 --- a/test/elli_static_tests.erl +++ b/test/elli_static_tests.erl @@ -7,7 +7,10 @@ elli_static_test_() -> fun setup/0, fun teardown/1, [?_test(readme()), ?_test(no_file()), - ?_test(not_found())]}. + ?_test(not_found()), + ?_test(safe_traversal()), + ?_test(unsafe_traversal()), + ?_test(invalid_path_separator())]}. readme() -> @@ -26,9 +29,41 @@ no_file() -> not_found() -> {ok, Response} = httpc:request("http://localhost:3000/not_found"), + ?assertMatch({{"HTTP/1.1",404,"Not Found"}, _Headers, "Not Found"}, Response), + + %% return 404 if the user doesn't provide a path + {ok, Response} = httpc:request("http://localhost:3000"), ?assertMatch({{"HTTP/1.1",404,"Not Found"}, _Headers, "Not Found"}, Response). +safe_traversal() -> + {ok, File} = file:read_file("README.md"), + Expected = binary_to_list(File), + + {ok, Response} = httpc:request("http://localhost:3000/elli_static/" + "../elli_static/README.md"), + ?assertEqual([integer_to_list(iolist_size(Expected))], + proplists:get_all_values("content-length", element(2, Response))), + ?assertMatch({_Status, _Headers, Expected}, Response), + + %% `Response' should match the same request above + {ok, Response} = httpc:request("http://localhost:3000/elli_static/./README.md"). + +unsafe_traversal() -> + %% compute the relative path to /etc/passwd + {ok, Cwd} = file:get_cwd(), + PasswdPath = [".." || _ <- filename:split(Cwd)] ++ ["etc", "passwd"], + Path = filename:join(PasswdPath), + + {ok, Response} = httpc:request("http://localhost:3000/elli_static/" ++ Path), + ?assertMatch({{"HTTP/1.1",404,"Not Found"}, _Headers, "Not Found"}, Response). + +invalid_path_separator() -> + %% https://www.ietf.org/rfc/rfc2396.txt defines a path separator to be a + %% single slash + {ok, Response} = httpc:request("http://localhost:3000////elli_static/README.md"), + ?assertMatch({{"HTTP/1.1",404,"Not Found"}, _Headers, "Not Found"}, Response). + setup() -> {ok, Dir} = file:get_cwd(), Args = [{<<"/elli_static">>, {dir, Dir}}], diff --git a/test/elli_static_utils_tests.erl b/test/elli_static_utils_tests.erl new file mode 100644 index 0000000..e440b10 --- /dev/null +++ b/test/elli_static_utils_tests.erl @@ -0,0 +1,45 @@ +-module(elli_static_utils_tests). +-include_lib("eunit/include/eunit.hrl"). +-compile(export_all). + +elli_static_utils_test_() -> + [?_test(safe_relative_path())]. + +safe_relative_path() -> + %% lists + ?assertEqual("a", elli_static_utils:safe_relative_path("a")), + ?assertEqual("a/b", elli_static_utils:safe_relative_path("a/b")), + ?assertEqual("a/b", elli_static_utils:safe_relative_path("a/./b")), + ?assertEqual("a/b", elli_static_utils:safe_relative_path("a/./b/.")), + ?assertEqual("", elli_static_utils:safe_relative_path("a/..")), + ?assertEqual("", elli_static_utils:safe_relative_path("a/./..")), + ?assertEqual("", elli_static_utils:safe_relative_path("a/../.")), + ?assertEqual("a", elli_static_utils:safe_relative_path("a/b/..")), + ?assertEqual("a", elli_static_utils:safe_relative_path("a/../a")), + ?assertEqual("a", elli_static_utils:safe_relative_path("a/../a/../a")), + ?assertEqual("a/b/c", elli_static_utils:safe_relative_path("a/../a/b/c")), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path("a/../..")), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path("a/../../..")), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path("a/./../..")), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path("a/././../../..")), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path("a/b/././../../..")), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path("/root")), + + %% binaries + ?assertEqual(<<"a">>, elli_static_utils:safe_relative_path(<<"a">>)), + ?assertEqual(<<"a/b">>, elli_static_utils:safe_relative_path(<<"a/b">>)), + ?assertEqual(<<"a/b">>, elli_static_utils:safe_relative_path(<<"a/./b">>)), + ?assertEqual(<<"a/b">>, elli_static_utils:safe_relative_path(<<"a/./b/.">>)), + ?assertEqual([], elli_static_utils:safe_relative_path(<<"a/..">>)), + ?assertEqual([], elli_static_utils:safe_relative_path(<<"a/./..">>)), + ?assertEqual([], elli_static_utils:safe_relative_path(<<"a/../.">>)), + ?assertEqual(<<"a">>, elli_static_utils:safe_relative_path(<<"a/b/..">>)), + ?assertEqual(<<"a">>, elli_static_utils:safe_relative_path(<<"a/../a">>)), + ?assertEqual(<<"a">>, elli_static_utils:safe_relative_path(<<"a/../a/../a">>)), + ?assertEqual(<<"a/b/c">>, elli_static_utils:safe_relative_path(<<"a/../a/b/c">>)), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path(<<"a/../..">>)), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path(<<"a/../../..">>)), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path(<<"a/./../..">>)), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path(<<"a/././../../..">>)), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path(<<"a/b/././../../..">>)), + ?assertEqual(unsafe, elli_static_utils:safe_relative_path(<<"/root">>)).