File 0121-ssh-Reject-relative-paths-in-SFTP-root-option.patch of Package erlang
From 0e5912f22aedfe25bc0f0881237eae1207ff611b Mon Sep 17 00:00:00 2001
From: Jakub Witczak <kuba@erlang.org>
Date: Mon, 9 Mar 2026 15:38:09 +0100
Subject: [PATCH] ssh: Reject relative paths in SFTP root option
Relative paths in the SFTP root option cause unpredictable behavior
as file operations resolve relative to the Erlang VM's current
working directory instead of the intended root.
Add validation at daemon startup to reject relative root paths with
a clear error message. The root option must be an absolute path or
empty string.
Changes:
- Validate SFTP root option during daemon startup
- Provide detailed error message when validation fails
- Update documentation to state root must be absolute path
- Add test case verifying relative paths are rejected
---
lib/ssh/src/ssh_options.erl | 18 ++++++++++++---
lib/ssh/src/ssh_sftpd.erl | 24 ++++++++++----------
lib/ssh/test/ssh_sftpd_SUITE.erl | 39 ++++++++++++++++++++++++++++----
3 files changed, 61 insertions(+), 20 deletions(-)
diff --git a/lib/ssh/src/ssh_options.erl b/lib/ssh/src/ssh_options.erl
index 4f32e320ef..2006b2cc93 100644
--- a/lib/ssh/src/ssh_options.erl
+++ b/lib/ssh/src/ssh_options.erl
@@ -408,10 +408,11 @@ default(server) ->
#{default => [ssh_sftpd:subsystem_spec([])],
chk => fun(L) ->
is_list(L) andalso
- lists:all(fun({Name,{CB,Args}}) ->
+ lists:all(fun(SubSystem = {Name,{CB,Args}}) ->
check_string(Name) andalso
is_atom(CB) andalso
- is_list(Args);
+ is_list(Args) andalso
+ check_subsystem(SubSystem);
(_) ->
false
end, L)
@@ -1334,4 +1335,15 @@ error_if_empty([_|T]) ->
error_if_empty([]) ->
ok.
-%%%----------------------------------------------------------------
+check_subsystem({"sftp", {ssh_sftpd, Args}}) ->
+ Root = proplists:get_value(root, Args, ""),
+ case Root =:= "" orelse filename:pathtype(Root) =:= absolute of
+ true ->
+ true;
+ false ->
+ error_in_check(
+ {"sftp", {ssh_sftpd, Args}},
+ io_lib:format("SFTP root option must be an absolute path, got: ~p", [Root]))
+ end;
+check_subsystem(_) ->
+ true.
diff --git a/lib/ssh/src/ssh_sftpd.erl b/lib/ssh/src/ssh_sftpd.erl
index 8aca4ab037..2d222ba341 100644
--- a/lib/ssh/src/ssh_sftpd.erl
+++ b/lib/ssh/src/ssh_sftpd.erl
@@ -102,10 +102,10 @@ Options:
data provided by the SFTP client. (Note: limitations might be also
enforced by underlying operating system)
-- **`root`** - Sets the SFTP root directory. The user cannot access files
- outside this directory tree. If, for example, the root directory is set to
- `/tmp`, then the user sees this directory as `/`. If the user then writes
- `cd /etc`, the user moves to `/tmp/etc`.
+- **`root`** - Sets the SFTP root directory. Must be an absolute path (e.g., `/tmp`).
+ Then the user cannot see any files above this root. If, for example, the root
+ directory is set to `/tmp`, then the user sees this directory as `/`. If the
+ user then writes `cd /etc`, the user moves to `/tmp/etc`.
Note: This provides application-level isolation. For additional security,
consider using OS-level chroot or similar mechanisms. See the
@@ -144,14 +144,14 @@ subsystem_spec(Options) ->
%%--------------------------------------------------------------------
-doc false.
init(Options) ->
- {FileMod, FS0} = case proplists:get_value(file_handler, Options,
- {ssh_sftpd_file,[]}) of
- {F, S} ->
- {F, S};
- F ->
- {F, []}
- end,
-
+ {FileMod, FS0} =
+ case proplists:get_value(file_handler, Options,
+ {ssh_sftpd_file,[]}) of
+ {F, S} ->
+ {F, S};
+ F ->
+ {F, []}
+ end,
{{ok, Default}, FS1} = FileMod:get_cwd(FS0),
CWD = proplists:get_value(cwd, Options, Default),
diff --git a/lib/ssh/test/ssh_sftpd_SUITE.erl b/lib/ssh/test/ssh_sftpd_SUITE.erl
index 1cc36de387..85a704199c 100644
--- a/lib/ssh/test/ssh_sftpd_SUITE.erl
+++ b/lib/ssh/test/ssh_sftpd_SUITE.erl
@@ -35,7 +35,9 @@
end_per_testcase/2
]).
--export([access_outside_root/1,
+-export([
+ access_outside_root/1,
+ relative_root/1,
links/1,
mk_rm_dir/1,
open_close_dir/1,
@@ -103,6 +105,7 @@ all() ->
relpath,
ver6_basic,
access_outside_root,
+ relative_root,
root_with_cwd,
relative_path,
open_file_dir_v5,
@@ -141,6 +144,10 @@ end_per_group(_GroupName, Config) ->
%%--------------------------------------------------------------------
+init_per_testcase(relative_root, Config) ->
+ ssh:start(),
+ prep(Config),
+ Config;
init_per_testcase(TestCase, Config) ->
ssh:start(),
prep(Config),
@@ -222,12 +229,14 @@ init_per_testcase(TestCase, Config) ->
[{sftp, {Cm, Channel}}, {sftpd, Sftpd }| Config].
+end_per_testcase(relative_root, Config) ->
+ ssh:stop();
end_per_testcase(_TestCase, Config) ->
try
ssh:stop_daemon(proplists:get_value(sftpd, Config))
catch
- Class:Error:_Stack ->
- ?CT_LOG("Class = ~p Error = ~p", [Class, Error])
+ C:E:St ->
+ ?CT_LOG("~p:~p~n~p", [C, E, St])
end,
{Cm, Channel} = proplists:get_value(sftp, Config),
ssh_connection:close(Cm, Channel),
@@ -694,8 +703,10 @@ ver6_basic(Config) when is_list(Config) ->
%%--------------------------------------------------------------------
access_outside_root(Config) when is_list(Config) ->
PrivDir = proplists:get_value(priv_dir, Config),
- BaseDir = filename:join(PrivDir, access_outside_root),
- BadFilePath = filename:join([BaseDir, bad]),
+ BaseDir = filename:join(PrivDir, ?FUNCTION_NAME),
+ %% A file outside the tree below RootDir which is BaseDir/a
+ %% Make the file BaseDir/bad :
+ BadFilePath = filename:join([BaseDir, "bad.txt"]),
ok = file:write_file(BadFilePath, <<>>),
FileInSiblingDir = filename:join([BaseDir, a2, "secret.txt"]),
ok = filelib:ensure_dir(FileInSiblingDir),
@@ -719,6 +730,24 @@ access_outside_root(Config) when is_list(Config) ->
try_access("/../a2/secret.txt", Cm, Channel, 2),
try_access("../../a2/secret.txt", Cm, Channel, 3).
+relative_root(Config) when is_list(Config) ->
+ PrivDir = proplists:get_value(priv_dir, Config),
+ %% ClientUserDir = filename:join(PrivDir, nopubkey),
+ SystemDir = filename:join(proplists:get_value(priv_dir, Config), system),
+ Options = [{system_dir, SystemDir},
+ {user_dir, PrivDir},
+ {user_passwords,[{?USER, ?PASSWD}]},
+ {pwdfun, fun(_,_) -> true end}],
+ RootDir = "a/b",
+ SubSystems = [ssh_sftpd:subsystem_spec([{root, RootDir}])],
+ ExpectedErrMsg = "SFTP root option must be an absolute path, got: \"" ++ RootDir ++ "\"",
+ ?assertMatch(
+ {error, {eoptions,
+ {{subsystems, {"sftp", {ssh_sftpd,[{root, RootDir}]}}},
+ ExpectedErrMsg}}},
+ ssh:daemon(0, [{subsystems, SubSystems}|Options])),
+ ok.
+
try_access(Path, Cm, Channel, ReqId) ->
Return =
open_file(Path, Cm, Channel, ReqId,
--
2.51.0
