|
| 1 | +From ebf8ce14f3a42749982f8abbacbf27f8d16b3305 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Jakub Witczak <kuba@erlang.org> |
| 3 | +Date: Fri, 27 Feb 2026 12:24:47 +0100 |
| 4 | +Subject: [PATCH] ssh: Fix path traversal vulnerability in ssh_sftpd root |
| 5 | + directory validation |
| 6 | + |
| 7 | +The is_within_root/2 function used string prefix matching via |
| 8 | +lists:prefix/2, which allowed access to sibling directories with |
| 9 | +matching name prefixes (e.g., /tmp/root2/ when root is /tmp/root/). |
| 10 | + |
| 11 | +Changed to use path component-based validation with filename:split/1 |
| 12 | +to ensure proper directory containment checking. |
| 13 | + |
| 14 | +Added test cases for sibling directory bypass attempts in |
| 15 | +access_outside_root/1 test case. |
| 16 | + |
| 17 | +Security impact: Prevents authenticated SFTP users from escaping |
| 18 | +their configured root directory jail via sibling directory access. |
| 19 | + |
| 20 | +Signed-off-by: Azure Linux Security Servicing Account <azurelinux-security@microsoft.com> |
| 21 | +Upstream-reference: https://github.com/erlang/otp/commit/5ed603a1211b83b8be2d1fc06d3f3bf30c3c9759.patch |
| 22 | +--- |
| 23 | + lib/ssh/doc/src/hardening.xml | 18 +++++++++++++++ |
| 24 | + lib/ssh/doc/src/ssh_sftpd.xml | 13 +++++++---- |
| 25 | + lib/ssh/src/ssh_sftpd.erl | 12 +++++++++- |
| 26 | + lib/ssh/test/ssh_sftpd_SUITE.erl | 39 +++++++++++++++++++++++--------- |
| 27 | + 4 files changed, 65 insertions(+), 17 deletions(-) |
| 28 | + |
| 29 | +diff --git a/lib/ssh/doc/src/hardening.xml b/lib/ssh/doc/src/hardening.xml |
| 30 | +index cc530ac..0869be8 100644 |
| 31 | +--- a/lib/ssh/doc/src/hardening.xml |
| 32 | ++++ b/lib/ssh/doc/src/hardening.xml |
| 33 | +@@ -293,4 +293,22 @@ end. |
| 34 | + </p> |
| 35 | + </section> |
| 36 | + |
| 37 | ++ <section> |
| 38 | ++ <title>SFTP Security</title> |
| 39 | ++ <section> |
| 40 | ++ <title>Root Directory Isolation</title> |
| 41 | ++ <p>The <seeerl marker="ssh_sftpd"><c>root</c></seeerl> |
| 42 | ++ option restricts SFTP users to a specific directory tree, |
| 43 | ++ preventing access to files outside that directory. For example:</p> |
| 44 | ++ <code>ssh:daemon(Port, [{subsystems, [ssh_sftpd:subsystem_spec([{root, "/home/sftpuser"}])]}, ...]).</code> |
| 45 | ++ <p>Important: The <c>root</c> option is configured per daemon, not per user. All |
| 46 | ++ users connecting to the same daemon share the same root directory. For per-user |
| 47 | ++ isolation, consider running separate daemon instances on different ports or |
| 48 | ++ using OS-level mechanisms (PAM chroot, containers, file permissions).</p> |
| 49 | ++ <p>Defense-in-depth: For high-security deployments, combine the `root` option |
| 50 | ++ with OS-level isolation mechanisms such as chroot jails, containers, or |
| 51 | ++ mandatory access control (SELinux, AppArmor).</p> |
| 52 | ++ </section> |
| 53 | ++ </section> |
| 54 | ++ |
| 55 | + </chapter> |
| 56 | +diff --git a/lib/ssh/doc/src/ssh_sftpd.xml b/lib/ssh/doc/src/ssh_sftpd.xml |
| 57 | +index cbe015f..2bd4918 100644 |
| 58 | +--- a/lib/ssh/doc/src/ssh_sftpd.xml |
| 59 | ++++ b/lib/ssh/doc/src/ssh_sftpd.xml |
| 60 | +@@ -79,11 +79,14 @@ |
| 61 | + </item> |
| 62 | + <tag><c>root</c></tag> |
| 63 | + <item> |
| 64 | +- <p>Sets the SFTP root directory. Then the user cannot see any files |
| 65 | +- above this root. If, for example, the root directory is set to <c>/tmp</c>, |
| 66 | +- then the user sees this directory as <c>/</c>. If the user then writes |
| 67 | +- <c>cd /etc</c>, the user moves to <c>/tmp/etc</c>. |
| 68 | +- </p> |
| 69 | ++ <p>Sets the SFTP root directory. The user cannot access files |
| 70 | ++ outside this directory tree. If, for example, the root directory is set to |
| 71 | ++ <c>/tmp</c>, then the user sees this directory as <c>/</c>. If the user then writes |
| 72 | ++ <c>cd /etc</c>, the user moves to <c>/tmp/etc</c>.</p> |
| 73 | ++ <p>Note: This provides application-level isolation. For additional security, |
| 74 | ++ consider using OS-level chroot or similar mechanisms. See the |
| 75 | ++ <seeguide marker="hardening#sftp-security">SFTP Security</seeguide> |
| 76 | ++ section in the Hardening guide for deployment recommendations.</p> |
| 77 | + </item> |
| 78 | + <tag><c>sftpd_vsn</c></tag> |
| 79 | + <item> |
| 80 | +diff --git a/lib/ssh/src/ssh_sftpd.erl b/lib/ssh/src/ssh_sftpd.erl |
| 81 | +index d02ece3..77fa495 100644 |
| 82 | +--- a/lib/ssh/src/ssh_sftpd.erl |
| 83 | ++++ b/lib/ssh/src/ssh_sftpd.erl |
| 84 | +@@ -871,7 +871,17 @@ relate_file_name(File, #state{cwd = CWD, root = Root}, Canonicalize) -> |
| 85 | + end. |
| 86 | + |
| 87 | + is_within_root(Root, File) -> |
| 88 | +- lists:prefix(Root, File). |
| 89 | ++ RootParts = filename:split(Root), |
| 90 | ++ FileParts = filename:split(File), |
| 91 | ++ is_prefix_components(RootParts, FileParts). |
| 92 | ++ |
| 93 | ++%% Verify if request file path is within configured root directory |
| 94 | ++is_prefix_components([], _) -> |
| 95 | ++ true; |
| 96 | ++is_prefix_components([H|T1], [H|T2]) -> |
| 97 | ++ is_prefix_components(T1, T2); |
| 98 | ++is_prefix_components(_, _) -> |
| 99 | ++ false. |
| 100 | + |
| 101 | + %% Remove leading slash (/), if any, in order to make the filename |
| 102 | + %% relative (to the root) |
| 103 | +diff --git a/lib/ssh/test/ssh_sftpd_SUITE.erl b/lib/ssh/test/ssh_sftpd_SUITE.erl |
| 104 | +index 01321ed..b4ceb02 100644 |
| 105 | +--- a/lib/ssh/test/ssh_sftpd_SUITE.erl |
| 106 | ++++ b/lib/ssh/test/ssh_sftpd_SUITE.erl |
| 107 | +@@ -33,8 +33,7 @@ |
| 108 | + end_per_testcase/2 |
| 109 | + ]). |
| 110 | + |
| 111 | +--export([ |
| 112 | +- access_outside_root/1, |
| 113 | ++-export([access_outside_root/1, |
| 114 | + links/1, |
| 115 | + mk_rm_dir/1, |
| 116 | + open_close_dir/1, |
| 117 | +@@ -160,7 +159,7 @@ init_per_testcase(TestCase, Config) -> |
| 118 | + RootDir = filename:join(BaseDir, a), |
| 119 | + CWD = filename:join(RootDir, b), |
| 120 | + %% Make the directory chain: |
| 121 | +- ok = filelib:ensure_dir(filename:join(CWD, tmp)), |
| 122 | ++ ok = filelib:ensure_path(CWD), |
| 123 | + SubSystems = [ssh_sftpd:subsystem_spec([{root, RootDir}, |
| 124 | + {cwd, CWD}])], |
| 125 | + ssh:daemon(0, [{subsystems, SubSystems}|Options]); |
| 126 | +@@ -221,7 +220,12 @@ init_per_testcase(TestCase, Config) -> |
| 127 | + [{sftp, {Cm, Channel}}, {sftpd, Sftpd }| Config]. |
| 128 | + |
| 129 | + end_per_testcase(_TestCase, Config) -> |
| 130 | +- catch ssh:stop_daemon(proplists:get_value(sftpd, Config)), |
| 131 | ++ try |
| 132 | ++ ssh:stop_daemon(proplists:get_value(sftpd, Config)) |
| 133 | ++ catch |
| 134 | ++ Class:Error:_Stack -> |
| 135 | ++ ct:log("Class = ~p Error = ~p", [Class, Error]) |
| 136 | ++ end, |
| 137 | + {Cm, Channel} = proplists:get_value(sftp, Config), |
| 138 | + ssh_connection:close(Cm, Channel), |
| 139 | + ssh:close(Cm), |
| 140 | +@@ -687,25 +691,38 @@ ver6_basic(Config) when is_list(Config) -> |
| 141 | + access_outside_root(Config) when is_list(Config) -> |
| 142 | + PrivDir = proplists:get_value(priv_dir, Config), |
| 143 | + BaseDir = filename:join(PrivDir, access_outside_root), |
| 144 | +- %% A file outside the tree below RootDir which is BaseDir/a |
| 145 | +- %% Make the file BaseDir/bad : |
| 146 | + BadFilePath = filename:join([BaseDir, bad]), |
| 147 | + ok = file:write_file(BadFilePath, <<>>), |
| 148 | ++ FileInSiblingDir = filename:join([BaseDir, a2, "secret.txt"]), |
| 149 | ++ ok = filelib:ensure_dir(FileInSiblingDir), |
| 150 | ++ ok = file:write_file(FileInSiblingDir, <<"secret">>), |
| 151 | ++ TestFolderStructure = |
| 152 | ++ <<"PrivDir |
| 153 | ++ |-- access_outside_root (BaseDir) |
| 154 | ++ | |-- a (RootDir folder) |
| 155 | ++ | | +-- b (CWD folder) |
| 156 | ++ | |-- a2 (sibling folder with name prefix equal to RootDir) |
| 157 | ++ | | +-- secret.txt |
| 158 | ++ | +-- bad.txt">>, |
| 159 | ++ ct:log("TestFolderStructure = ~n~s", [TestFolderStructure]), |
| 160 | + {Cm, Channel} = proplists:get_value(sftp, Config), |
| 161 | +- %% Try to access a file parallel to the RootDir: |
| 162 | +- try_access("/../bad", Cm, Channel, 0), |
| 163 | ++ %% Try to access a file parallel to the RootDir using parent traversal: |
| 164 | ++ try_access("/../bad.txt", Cm, Channel, 0), |
| 165 | + %% Try to access the same file via the CWD which is /b relative to the RootDir: |
| 166 | +- try_access("../../bad", Cm, Channel, 1). |
| 167 | +- |
| 168 | ++ try_access("../../bad.txt", Cm, Channel, 1), |
| 169 | ++ %% Try to access sibling folder name prefixed with root dir |
| 170 | ++ try_access("/../a2/secret.txt", Cm, Channel, 2), |
| 171 | ++ try_access("../../a2/secret.txt", Cm, Channel, 3). |
| 172 | + |
| 173 | + try_access(Path, Cm, Channel, ReqId) -> |
| 174 | + Return = |
| 175 | + open_file(Path, Cm, Channel, ReqId, |
| 176 | + ?ACE4_READ_DATA bor ?ACE4_READ_ATTRIBUTES, |
| 177 | + ?SSH_FXF_OPEN_EXISTING), |
| 178 | +- ct:log("Try open ~p -> ~p",[Path,Return]), |
| 179 | ++ ct:log("Try open ~p -> ~w",[Path,Return]), |
| 180 | + case Return of |
| 181 | + {ok, <<?SSH_FXP_HANDLE, ?UINT32(ReqId), _Handle0/binary>>, _} -> |
| 182 | ++ ct:log("Got the unexpected ?SSH_FXP_HANDLE",[]), |
| 183 | + ct:fail("Could open a file outside the root tree!"); |
| 184 | + {ok, <<?SSH_FXP_STATUS, ?UINT32(ReqId), ?UINT32(Code), Rest/binary>>, <<>>} -> |
| 185 | + case Code of |
| 186 | +-- |
| 187 | +2.45.4 |
| 188 | + |
0 commit comments