From 69a589ff02270a11a2fb3ba8eabb82fe34142d43 Mon Sep 17 00:00:00 2001 From: Jakub Witczak Date: Fri, 27 Feb 2026 12:24:47 +0100 Subject: [PATCH] ssh: Fix path traversal vulnerability in ssh_sftpd root directory validation The is_within_root/2 function used string prefix matching via lists:prefix/2, which allowed access to sibling directories with matching name prefixes (e.g., /tmp/root2/ when root is /tmp/root/). Changed to use path component-based validation with filename:split/1 to ensure proper directory containment checking. Added test cases for sibling directory bypass attempts in access_outside_root/1 test case. Security impact: Prevents authenticated SFTP users from escaping their configured root directory jail via sibling directory access. --- lib/ssh/doc/src/hardening.xml | 18 +++++++++++++++ lib/ssh/doc/src/ssh_sftpd.xml | 13 +++++++---- lib/ssh/src/ssh_sftpd.erl | 12 +++++++++- lib/ssh/test/ssh_sftpd_SUITE.erl | 39 +++++++++++++++++++++++--------- 4 files changed, 65 insertions(+), 17 deletions(-) Index: otp-OTP-23.3.4.19/lib/ssh/doc/src/hardening.xml =================================================================== --- otp-OTP-23.3.4.19.orig/lib/ssh/doc/src/hardening.xml +++ otp-OTP-23.3.4.19/lib/ssh/doc/src/hardening.xml @@ -272,4 +272,22 @@ end.

+
+ SFTP Security +
+ Root Directory Isolation +

The root + option restricts SFTP users to a specific directory tree, + preventing access to files outside that directory. For example:

+ ssh:daemon(Port, [{subsystems, [ssh_sftpd:subsystem_spec([{root, "/home/sftpuser"}])]}, ...]). +

Important: The root option is configured per daemon, not per user. All + users connecting to the same daemon share the same root directory. For per-user + isolation, consider running separate daemon instances on different ports or + using OS-level mechanisms (PAM chroot, containers, file permissions).

+

Defense-in-depth: For high-security deployments, combine the `root` option + with OS-level isolation mechanisms such as chroot jails, containers, or + mandatory access control (SELinux, AppArmor).

+
+
+ Index: otp-OTP-23.3.4.19/lib/ssh/doc/src/ssh_sftpd.xml =================================================================== --- otp-OTP-23.3.4.19.orig/lib/ssh/doc/src/ssh_sftpd.xml +++ otp-OTP-23.3.4.19/lib/ssh/doc/src/ssh_sftpd.xml @@ -79,11 +79,14 @@ root -

Sets the SFTP root directory. 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. -

+

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.

+

Note: This provides application-level isolation. For additional security, + consider using OS-level chroot or similar mechanisms. See the + SFTP Security + section in the Hardening guide for deployment recommendations.

 sftpd_vsn Index: otp-OTP-23.3.4.19/lib/ssh/src/ssh_sftpd.erl =================================================================== --- otp-OTP-23.3.4.19.orig/lib/ssh/src/ssh_sftpd.erl +++ otp-OTP-23.3.4.19/lib/ssh/src/ssh_sftpd.erl @@ -877,7 +877,17 @@ relate_file_name(File, #state{cwd = CWD, end. is_within_root(Root, File) -> - lists:prefix(Root, File). + RootParts = filename:split(Root), + FileParts = filename:split(File), + is_prefix_components(RootParts, FileParts). + +%% Verify if request file path is within configured root directory +is_prefix_components([], _) -> + true; +is_prefix_components([H|T1], [H|T2]) -> + is_prefix_components(T1, T2); +is_prefix_components(_, _) -> + false. %% Remove leading slash (/), if any, in order to make the filename %% relative (to the root) Index: otp-OTP-23.3.4.19/lib/ssh/test/ssh_sftpd_SUITE.erl =================================================================== --- otp-OTP-23.3.4.19.orig/lib/ssh/test/ssh_sftpd_SUITE.erl +++ otp-OTP-23.3.4.19/lib/ssh/test/ssh_sftpd_SUITE.erl @@ -33,8 +33,7 @@ end_per_testcase/2 ]). --export([ - access_outside_root/1, +-export([access_outside_root/1, links/1, mk_rm_dir/1, open_close_dir/1, @@ -161,7 +160,7 @@ init_per_testcase(TestCase, Config) -> RootDir = filename:join(BaseDir, a), CWD = filename:join(RootDir, b), %% Make the directory chain: - ok = filelib:ensure_dir(filename:join(CWD, tmp)), + ok = filelib:ensure_path(CWD), SubSystems = [ssh_sftpd:subsystem_spec([{root, RootDir}, {cwd, CWD}])], ssh:daemon(0, [{subsystems, SubSystems}|Options]); @@ -222,7 +221,12 @@ init_per_testcase(TestCase, Config) -> [{sftp, {Cm, Channel}}, {sftpd, Sftpd }| Config]. end_per_testcase(_TestCase, Config) -> - catch ssh:stop_daemon(proplists:get_value(sftpd, Config)), + try + ssh:stop_daemon(proplists:get_value(sftpd, Config)) + catch + Class:Error:_Stack -> + ct:log("Class = ~p Error = ~p", [Class, Error]) + end, {Cm, Channel} = proplists:get_value(sftp, Config), ssh_connection:close(Cm, Channel), ssh:close(Cm), @@ -689,25 +693,38 @@ 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), - %% A file outside the tree below RootDir which is BaseDir/a - %% Make the file BaseDir/bad : BadFilePath = filename:join([BaseDir, bad]), ok = file:write_file(BadFilePath, <<>>), + FileInSiblingDir = filename:join([BaseDir, a2, "secret.txt"]), + ok = filelib:ensure_dir(FileInSiblingDir), + ok = file:write_file(FileInSiblingDir, <<"secret">>), + TestFolderStructure = + <<"PrivDir + |-- access_outside_root (BaseDir) + | |-- a (RootDir folder) + | | +-- b (CWD folder) + | |-- a2 (sibling folder with name prefix equal to RootDir) + | | +-- secret.txt + | +-- bad.txt">>, + ct:log("TestFolderStructure = ~n~s", [TestFolderStructure]), {Cm, Channel} = proplists:get_value(sftp, Config), - %% Try to access a file parallel to the RootDir: - try_access("/../bad", Cm, Channel, 0), + %% Try to access a file parallel to the RootDir using parent traversal: + try_access("/../bad.txt", Cm, Channel, 0), %% Try to access the same file via the CWD which is /b relative to the RootDir: - try_access("../../bad", Cm, Channel, 1). - + try_access("../../bad.txt", Cm, Channel, 1), + %% Try to access sibling folder name prefixed with root dir + try_access("/../a2/secret.txt", Cm, Channel, 2), + try_access("../../a2/secret.txt", Cm, Channel, 3). try_access(Path, Cm, Channel, ReqId) -> Return = open_file(Path, Cm, Channel, ReqId, ?ACE4_READ_DATA bor ?ACE4_READ_ATTRIBUTES, ?SSH_FXF_OPEN_EXISTING), - ct:log("Try open ~p -> ~p",[Path,Return]), + ct:log("Try open ~p -> ~w",[Path,Return]), case Return of {ok, <>, _} -> + ct:log("Got the unexpected ?SSH_FXP_HANDLE",[]), ct:fail("Could open a file outside the root tree!"); {ok, <>, <<>>} -> case Code of