From 4427108b69fcd1e6a8233a217fa0e99d0564b714 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= <essen@ninenines.eu>
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 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 27 deletions(-)

(limited to 'src/cowboy_static.erl')

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}.
-- 
cgit v1.2.3