From 28c5d5a6c5f873dc701b597276271763e7d1c004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=C4=85sowski?= Date: Mon, 20 Apr 2026 19:16:52 +0200 Subject: [PATCH] Fix root escape vulnerability in SSH_FXP_FSETSTAT --- lib/ssh/src/ssh_sftpd.erl | 62 +++++++------- lib/ssh/test/ssh_sftpd_SUITE.erl | 138 ++++++++++++++++++++++++------- 2 files changed, 139 insertions(+), 61 deletions(-) Index: otp-OTP-26.2.1/lib/ssh/src/ssh_sftpd.erl =================================================================== --- otp-OTP-26.2.1.orig/lib/ssh/src/ssh_sftpd.erl +++ otp-OTP-26.2.1/lib/ssh/src/ssh_sftpd.erl @@ -56,8 +56,8 @@ max_path, % integer > 0 - max length of path options, % from the subsystem declaration handles % list of open handles - %% handle is either {, directory, {Path, unread|eof}} or - %% {, file, {Path, IoDevice}} + %% handle is either {, directory, {AbsPath, unread|eof}} or + %% {, file, {AbsPath, IoDevice}} }). %%==================================================================== @@ -310,7 +310,7 @@ handle_op(?SSH_FXP_OPENDIR, ReqId, "Not a directory"), State1; true when HandlesCnt < MaxHandles -> - add_handle(State1, XF, ReqId, directory, {RelPath,unread}); + add_handle(State1, XF, ReqId, directory, {AbsPath,unread}); true -> ssh_xfer:xf_send_status(XF, ReqId, ?SSH_FX_FAILURE, "max_handles limit reached"), @@ -321,11 +321,11 @@ handle_op(?SSH_FXP_READDIR, ReqId, State) -> XF = State#state.xf, case get_handle(State#state.handles, BinHandle) of - {_Handle, directory, {_RelPath, eof}} -> + {_Handle, directory, {_AbsPath, eof}} -> ssh_xfer:xf_send_status(XF, ReqId, ?SSH_FX_EOF), State; - {Handle, directory, {RelPath, Status}} -> - read_dir(State, XF, ReqId, Handle, RelPath, Status); + {Handle, directory, {AbsPath, Status}} -> + read_dir(State, XF, ReqId, Handle, AbsPath, Status); _ -> ssh_xfer:xf_send_status(XF, ReqId, ?SSH_FX_INVALID_HANDLE), State @@ -361,7 +361,7 @@ handle_op(?SSH_FXP_READ, ReqId, <>, State) -> case get_handle(State#state.handles, BinHandle) of - {_Handle, file, {_Path, IoDevice}} -> + {_Handle, file, {_AbsPath, IoDevice}} -> read_file(ReqId, IoDevice, Offset, Len, State); _ -> ssh_xfer:xf_send_status(State#state.xf, ReqId, @@ -372,7 +372,7 @@ handle_op(?SSH_FXP_WRITE, ReqId, <>, State) -> case get_handle(State#state.handles, BinHandle) of - {_Handle, file, {_Path, IoDevice}} -> + {_Handle, file, {_AbsPath, IoDevice}} -> write_file(ReqId, IoDevice, Offset, Data, State); _ -> ssh_xfer:xf_send_status(State#state.xf, ReqId, @@ -415,8 +415,8 @@ handle_op(?SSH_FXP_FSETSTAT, ReqId, < case get_handle(Handles, BinHandle) of - {_Handle, _Type, {Path,_}} -> - {Status, State1} = set_stat(Attr, Path, State0), + {_Handle, _Type, {AbsPath,_}} -> + {Status, State1} = set_stat(Attr, AbsPath, State0), send_status(Status, ReqId, State1); _ -> ssh_xfer:xf_send_status(State0#state.xf, ReqId, @@ -526,8 +526,7 @@ get_handle(Handles, BinHandle) -> %%% read_dir/5: read directory, send names, and return new state read_dir(State0 = #state{file_handler = FileMod, max_files = MaxLength, file_state = FS0}, - XF = #ssh_xfer{cm = _CM, channel = _Channel, vsn = Vsn}, ReqId, Handle, RelPath, {cache, Files}) -> - AbsPath = relate_file_name(RelPath, State0), + XF = #ssh_xfer{cm = _CM, channel = _Channel, vsn = Vsn}, ReqId, Handle, AbsPath, {cache, Files}) -> if length(Files) > MaxLength -> {ToSend, NewCache} = lists:split(MaxLength, Files), @@ -535,19 +534,18 @@ read_dir(State0 = #state{file_handler = ssh_xfer:xf_send_names(XF, ReqId, NamesAndAttrs), Handles = lists:keyreplace(Handle, 1, State0#state.handles, - {Handle, directory, {RelPath,{cache, NewCache}}}), + {Handle, directory, {AbsPath,{cache, NewCache}}}), State0#state{handles = Handles, file_state = FS1}; true -> {NamesAndAttrs, FS1} = get_attrs(AbsPath, Files, FileMod, FS0, Vsn), ssh_xfer:xf_send_names(XF, ReqId, NamesAndAttrs), Handles = lists:keyreplace(Handle, 1, State0#state.handles, - {Handle, directory, {RelPath,eof}}), + {Handle, directory, {AbsPath,eof}}), State0#state{handles = Handles, file_state = FS1} end; read_dir(State0 = #state{file_handler = FileMod, max_files = MaxLength, file_state = FS0}, - XF = #ssh_xfer{cm = _CM, channel = _Channel, vsn = Vsn}, ReqId, Handle, RelPath, _Status) -> - AbsPath = relate_file_name(RelPath, State0), + XF = #ssh_xfer{cm = _CM, channel = _Channel, vsn = Vsn}, ReqId, Handle, AbsPath, _Status) -> {Res, FS1} = FileMod:list_dir(AbsPath, FS0), case Res of {ok, Files} when MaxLength == 0 orelse MaxLength > length(Files) -> @@ -555,7 +553,7 @@ read_dir(State0 = #state{file_handler = ssh_xfer:xf_send_names(XF, ReqId, NamesAndAttrs), Handles = lists:keyreplace(Handle, 1, State0#state.handles, - {Handle, directory, {RelPath,eof}}), + {Handle, directory, {AbsPath,eof}}), State0#state{handles = Handles, file_state = FS2}; {ok, Files} -> {ToSend, Cache} = lists:split(MaxLength, Files), @@ -563,7 +561,7 @@ read_dir(State0 = #state{file_handler = ssh_xfer:xf_send_names(XF, ReqId, NamesAndAttrs), Handles = lists:keyreplace(Handle, 1, State0#state.handles, - {Handle, directory, {RelPath,{cache, Cache}}}), + {Handle, directory, {AbsPath,{cache, Cache}}}), State0#state{handles = Handles, file_state = FS2}; {error, Error} -> State1 = State0#state{file_state = FS1}, @@ -619,13 +617,13 @@ get_long_name(FileName, I) when is_recor I#file_info.mode, I#file_info.uid, I#file_info.gid}). %%% get_attrs: get stat of each file and return -get_attrs(RelPath, Files, FileMod, FS, Vsn) -> - get_attrs(RelPath, Files, FileMod, FS, Vsn, []). +get_attrs(AbsBase, Files, FileMod, FS, Vsn) -> + get_attrs(AbsBase, Files, FileMod, FS, Vsn, []). -get_attrs(_RelPath, [], _FileMod, FS, _Vsn, Acc) -> +get_attrs(_AbsBase, [], _FileMod, FS, _Vsn, Acc) -> {lists:reverse(Acc), FS}; -get_attrs(RelPath, [F | Rest], FileMod, FS0, Vsn, Acc) -> - Path = filename:absname(F, RelPath), +get_attrs(AbsBase, [F | Rest], FileMod, FS0, Vsn, Acc) -> + Path = filename:absname(F, AbsBase), case FileMod:read_link_info(Path, FS0) of {{ok, Info}, FS1} -> Name = if Vsn =< 3 -> @@ -635,12 +633,12 @@ get_attrs(RelPath, [F | Rest], FileMod, F end, Attrs = ssh_sftp:info_to_attr(Info), - get_attrs(RelPath, Rest, FileMod, FS1, Vsn, [{Name, Attrs} | Acc]); + get_attrs(AbsBase, Rest, FileMod, FS1, Vsn, [{Name, Attrs} | Acc]); {{error, Msg}, FS1} when Msg == enoent ; % The item has disappeared after reading the list of items to check Msg == eacces -> % You are not allowed to read this %% Skip this F and check the remaining Rest - get_attrs(RelPath, Rest, FileMod, FS1, Vsn, Acc); + get_attrs(AbsBase, Rest, FileMod, FS1, Vsn, Acc); {Error, FS1} -> {Error, FS1} end. @@ -663,23 +661,25 @@ fstat(Vsn, ReqId, Data, State) when Vsn fstat(ReqId, BinHandle, State) -> case get_handle(State#state.handles, BinHandle) of - {_Handle, _Type, {Path, _}} -> - stat(ReqId, Path, State, read_file_info); + {_Handle, _Type, {AbsPath, _}} -> + do_stat(ReqId, AbsPath, State, read_file_info); _ -> ssh_xfer:xf_send_status(State#state.xf, ReqId, ?SSH_FX_INVALID_HANDLE), State end. -stat(ReqId, RelPath, State0=#state{file_handler=FileMod, - file_state=FS0}, F) -> +stat(ReqId, RelPath, State0, F) -> AbsPath = relate_file_name(RelPath, State0), + do_stat(ReqId, AbsPath, State0, F). + +do_stat(ReqId, AbsPath, State0=#state{file_handler=FileMod, file_state=FS0}, F) -> XF = State0#state.xf, {Res, FS1} = FileMod:F(AbsPath, FS0), State1 = State0#state{file_state = FS1}, case Res of {ok, FileInfo} -> - ssh_xfer:xf_send_attr(XF, ReqId, + ssh_xfer:xf_send_attr(XF, ReqId, ssh_sftp:info_to_attr(FileInfo)), State1; {error, E} -> @@ -789,7 +789,7 @@ do_open(ReqId, State0, Path, Flags) -> State1 = State0#state{file_state = FS1}, case Res of {ok, IoDevice} -> - add_handle(State1, State0#state.xf, ReqId, file, {Path,IoDevice}); + add_handle(State1, State0#state.xf, ReqId, file, {AbsPath,IoDevice}); {error, Error} -> ssh_xfer:xf_send_status(State1#state.xf, ReqId, ssh_xfer:encode_erlang_status(Error)), Index: otp-OTP-26.2.1/lib/ssh/test/ssh_sftpd_SUITE.erl =================================================================== --- otp-OTP-26.2.1.orig/lib/ssh/test/ssh_sftpd_SUITE.erl +++ otp-OTP-26.2.1/lib/ssh/test/ssh_sftpd_SUITE.erl @@ -54,7 +54,8 @@ ver3_open_flags/1, ver3_rename/1, ver6_basic/1, - write_file/1 + write_file/1, + access_attributes_outside_root/1 ]). -include_lib("common_test/include/ct.hrl"). @@ -104,7 +105,8 @@ all() -> root_with_cwd, relative_path, open_file_dir_v5, - open_file_dir_v6]. + open_file_dir_v6, + access_attributes_outside_root]. groups() -> []. @@ -140,6 +142,7 @@ end_per_group(_GroupName, Config) -> %%-------------------------------------------------------------------- init_per_testcase(TestCase, Config) -> + {OsFamily, _} = os:type(), ssh:start(), prep(Config), PrivDir = proplists:get_value(priv_dir, Config), @@ -150,7 +153,7 @@ init_per_testcase(TestCase, Config) -> {user_dir, PrivDir}, {user_passwords,[{?USER, ?PASSWD}]}, {pwdfun, fun(_,_) -> true end}], - {ok, Sftpd} = case TestCase of + Result = case TestCase of ver6_basic -> SubSystems = [ssh_sftpd:subsystem_spec([{sftpd_vsn, 6}])], ssh:daemon(0, [{subsystems, SubSystems}|Options]); @@ -164,6 +167,14 @@ init_per_testcase(TestCase, Config) -> SubSystems = [ssh_sftpd:subsystem_spec([{root, RootDir}, {cwd, CWD}])], ssh:daemon(0, [{subsystems, SubSystems}|Options]); + access_attributes_outside_root when OsFamily =:= win32 -> + {skip, "Not implemented on windows"}; + access_attributes_outside_root -> + Rand = integer_to_list(rand:uniform(1000000)), + RootDir = filename:join("/tmp", Rand), + ok = file:make_dir(RootDir), + SubSystems = [ssh_sftpd:subsystem_spec([{root, RootDir}])], + ssh:daemon(0, [{subsystems, SubSystems}|Options]); root_with_cwd -> RootDir = filename:join(PrivDir, root_with_cwd), CWD = filename:join(RootDir, home), @@ -186,41 +197,59 @@ init_per_testcase(TestCase, Config) -> ssh:daemon(0, [{subsystems, SubSystems}|Options]) end, - Port = ssh_test_lib:daemon_port(Sftpd), - - Cm = ssh_test_lib:connect(Port, - [{user_dir, ClientUserDir}, - {user, ?USER}, {password, ?PASSWD}, - {user_interaction, false}, - {silently_accept_hosts, true}]), - {ok, Channel} = - ssh_connection:session_channel(Cm, ?XFER_WINDOW_SIZE, - ?XFER_PACKET_SIZE, ?SSH_TIMEOUT), - - success = ssh_connection:subsystem(Cm, Channel, "sftp", ?SSH_TIMEOUT), - - ProtocolVer = case atom_to_list(TestCase) of - "ver3_" ++ _ -> - 3; - _ -> - ?SSH_SFTP_PROTOCOL_VERSION - end, - - Data = <> , - - Size = 1 + size(Data), - - ssh_connection:send(Cm, Channel, << ?UINT32(Size), - ?SSH_FXP_INIT, Data/binary >>), - - {ok, <>, _} - = reply(Cm, Channel), - - ct:log("Client: ~p Server ~p~n", [ProtocolVer, Version]), - - [{sftp, {Cm, Channel}}, {sftpd, Sftpd }| Config]. + case Result of + {ok, Sftpd} -> + Port = ssh_test_lib:daemon_port(Sftpd), + + Cm = ssh_test_lib:connect(Port, + [{user_dir, ClientUserDir}, + {user, ?USER}, {password, ?PASSWD}, + {user_interaction, false}, + {silently_accept_hosts, true}]), + {ok, Channel} = + ssh_connection:session_channel(Cm, ?XFER_WINDOW_SIZE, + ?XFER_PACKET_SIZE, ?SSH_TIMEOUT), + + + success = ssh_connection:subsystem(Cm, Channel, "sftp", ?SSH_TIMEOUT), + + ProtocolVer = case atom_to_list(TestCase) of + "ver3_" ++ _ -> + 3; + _ -> + ?SSH_SFTP_PROTOCOL_VERSION + end, + + Data = <> , + + Size = 1 + size(Data), + ssh_connection:send(Cm, Channel, << ?UINT32(Size), + ?SSH_FXP_INIT, Data/binary >>), + + + {ok, <>, _} + = reply(Cm, Channel), + + ct:log("Client: ~p Server ~p~n", [ProtocolVer, Version]), + + [{sftp, {Cm, Channel}}, {sftpd, Sftpd }| Config]; + Other -> + Other + end. +end_per_testcase(access_attributes_outside_root, Config) -> + Sftpd = proplists:get_value(sftpd, Config), + {ok, DaemonInfo} = ssh:daemon_info(Sftpd), + ssh_cleanup(Config), + DaemonOpts = proplists:get_value(options, DaemonInfo), + Subsystems = proplists:get_value(subsystems, DaemonOpts), + {_, {_, SftpdOpts}} = lists:keyfind("sftp", 1, Subsystems), + RootDir = proplists:get_value(root, SftpdOpts), + file:del_dir_r(RootDir); end_per_testcase(_TestCase, Config) -> + ssh_cleanup(Config). + +ssh_cleanup(Config) -> try ssh:stop_daemon(proplists:get_value(sftpd, Config)) catch @@ -827,6 +856,53 @@ open_file_dir_v6(Config) when is_list(Co ?SSH_FXF_OPEN_EXISTING). %%-------------------------------------------------------------------- +access_attributes_outside_root(Config) when is_list(Config) -> + Sftpd = proplists:get_value(sftpd, Config), + {ok, DaemonInfo} = ssh:daemon_info(Sftpd), + DaemonOpts = proplists:get_value(options, DaemonInfo), + Subsystems = proplists:get_value(subsystems, DaemonOpts), + {_, {_, SftpdOpts}} = lists:keyfind("sftp", 1, Subsystems), + RootDir = proplists:get_value(root, SftpdOpts), + + TargetName = "target-" ++ filename:basename(RootDir) ++ ".txt", + InsideRootFile = filename:join([RootDir, "tmp", TargetName]), + ok = file:make_dir(filename:dirname(InsideRootFile)), + ok = file:write_file(InsideRootFile, <<"inside root">>), + {ok, InsideRootFileInfo} = file:read_file_info(InsideRootFile), + InsideRootFileMode = InsideRootFileInfo#file_info.mode, + + OutsideRootFile = filename:join("/tmp", TargetName), + ok = file:write_file(OutsideRootFile, <<"outside root">>), + try + {ok, OutsideRootFileInfo} = file:read_file_info(OutsideRootFile), + OutsideRootFileMode = OutsideRootFileInfo#file_info.mode, + + {Cm, Channel} = proplists:get_value(sftp, Config), + ReqId0 = 0, + {ok, <>, _} = + open_file(OutsideRootFile, Cm, Channel, ReqId0, + ?ACE4_READ_DATA bor ?ACE4_WRITE_ATTRIBUTES, + ?SSH_FXF_OPEN_EXISTING), + + Attrs = [?uint32(?SSH_FILEXFER_ATTR_PERMISSIONS), ?byte(?SSH_FILEXFER_TYPE_REGULAR), + ?uint32(not_default_permissions())], + + ReqId1 = 1, + {ok, <>, _} = + set_attributes_open_file(Handle, Attrs, Cm, Channel, ReqId1), + + {ok, NewOutsideRootFileInfo} = file:read_file_info(OutsideRootFile), + NewOutsideRootFileMode = NewOutsideRootFileInfo#file_info.mode, + ?assertEqual(OutsideRootFileMode, NewOutsideRootFileMode), + + {ok, NewInsideRootFileInfo} = file:read_file_info(InsideRootFile), + NewInsideRootFileMode = NewInsideRootFileInfo#file_info.mode, + ?assertNotEqual(InsideRootFileMode, NewInsideRootFileMode) + after + file:delete(OutsideRootFile) + end. + +%%-------------------------------------------------------------------- %% Internal functions ------------------------------------------------ %%-------------------------------------------------------------------- prep(Config) -> @@ -840,7 +916,9 @@ prep(Config) -> %% Initial config DataDir = proplists:get_value(data_dir, Config), FileName = filename:join(DataDir, "test.txt"), - file:copy(FileName, TestFile), + {ok, Data0} = file:read_file(FileName), + Data = ssh_test_lib:remove_comment(Data0), + ok = file:write_file(TestFile, string:chomp(Data)), Mode = 8#00400 bor 8#00200 bor 8#00040, % read & write owner, read group {ok, FileInfo} = file:read_file_info(TestFile), ok = file:write_file_info(TestFile,