Skip to content

Commit ea22e76

Browse files
committed
Updated protection of remove_all against CVE-2022-21658 on Windows.
This follows up the previous update for POSIX. The new implementation of remove_all on Windows Vista and later uses NtCreateFile internal function in order to open files relative to a previously opened directory handle, similar to POSIX openat. Furthermore, querying file status and removing the file is now also done through file handles to avoid performing path resolutions. Closes boostorg#224.
1 parent 36cf9aa commit ea22e76

File tree

6 files changed

+551
-191
lines changed

6 files changed

+551
-191
lines changed

doc/release_history.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ <h2>1.80.0</h2>
4545
<li>On Windows, added a workaround for FAT/exFAT filesystems that produce <code>ERROR_INVALID_PARAMETER</code> when querying file attributes. This affected <code>status</code> and <code>symlink_status</code>, which reported that files do not exist, and directory iterators, which failed to construct, as well as other dependent operations. (<a href="https://github.com/boostorg/filesystem/issues/236">#236</a>, <a href="https://github.com/boostorg/filesystem/issues/237">#237</a>)</li>
4646
<li>Worked around a compilation problem on <a href="https://www.rtems.org/">RTEMS</a>. (<a href="https://github.com/boostorg/filesystem/pull/240">#240</a>)</li>
4747
<li>On Linux, corrected switching to <code>sendfile</code> <code>copy_file</code> implementation if <code>copy_file_range</code> failed with <code>ENOSYS</code> in runtime. The <code>sendfile</code> fallback implementation used to skip the filesystem type check and could fail for some filesystems.</li>
48-
<li>On POSIX systems supporting <code>openat</code> and related APIs defined in POSIX.1-2008, improved protection of <code>remove_all</code> against <a href="https://www.cve.org/CVERecord?id=CVE-2022-21658">CVE-2022-21658</a> that was implemented in the previous release. The previous fix could still result in removing unintended files in <a href="https://github.com/boostorg/filesystem/issues/224#issuecomment-1183738097">certain conditions</a>. Other systems, including Windows, remain vulnerable.</li>
48+
<li>On POSIX systems supporting <code>openat</code> and related APIs defined in POSIX.1-2008 and on Windows Vista and later, improved protection of <code>remove_all</code> against <a href="https://www.cve.org/CVERecord?id=CVE-2022-21658">CVE-2022-21658</a> that was implemented in the previous release. The previous fix could still result in removing unintended files in <a href="https://github.com/boostorg/filesystem/issues/224#issuecomment-1183738097">certain conditions</a>. Other systems remain vulnerable.</li>
4949
</ul>
5050

5151
<h2>1.79.0</h2>

include/boost/filesystem/directory.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ struct dir_itr_imp :
263263
public boost::intrusive_ref_counter< dir_itr_imp >
264264
{
265265
#ifdef BOOST_WINDOWS_API
266+
bool close_handle;
266267
unsigned char extra_data_format;
267268
std::size_t current_offset;
268269
#endif
@@ -271,6 +272,7 @@ struct dir_itr_imp :
271272

272273
dir_itr_imp() BOOST_NOEXCEPT :
273274
#ifdef BOOST_WINDOWS_API
275+
close_handle(false),
274276
extra_data_format(0u),
275277
current_offset(0u),
276278
#endif

src/directory.cpp

Lines changed: 66 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,8 @@ inline system::error_code dir_itr_close(dir_itr_imp& imp) BOOST_NOEXCEPT
610610

611611
if (imp.handle != NULL)
612612
{
613-
::CloseHandle(imp.handle);
613+
if (BOOST_LIKELY(imp.close_handle))
614+
::CloseHandle(imp.handle);
614615
imp.handle = NULL;
615616
}
616617

@@ -762,62 +763,76 @@ error_code dir_itr_increment(dir_itr_imp& imp, fs::path& filename, fs::file_stat
762763
return error_code();
763764
}
764765

765-
error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::path const& dir, unsigned int opts, directory_iterator_params*, fs::path& first_filename, fs::file_status& sf, fs::file_status& symlink_sf)
766+
error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::path const& dir, unsigned int opts, directory_iterator_params* params, fs::path& first_filename, fs::file_status& sf, fs::file_status& symlink_sf)
766767
{
767768
boost::intrusive_ptr< detail::dir_itr_imp > pimpl(new (dir_itr_extra_size) detail::dir_itr_imp());
768769
if (BOOST_UNLIKELY(!pimpl))
769770
return make_error_code(system::errc::not_enough_memory);
770771

771-
DWORD flags = FILE_FLAG_BACKUP_SEMANTICS;
772-
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
773-
flags |= FILE_FLAG_OPEN_REPARSE_POINT;
774-
handle_wrapper h(create_file_handle(dir, FILE_LIST_DIRECTORY, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, flags));
775-
if (BOOST_UNLIKELY(h.handle == INVALID_HANDLE_VALUE))
772+
GetFileInformationByHandleEx_t* get_file_information_by_handle_ex = filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api);
773+
774+
handle_wrapper h;
775+
HANDLE iterator_handle;
776+
bool close_handle = true;
777+
if (params != NULL && params->use_handle != INVALID_HANDLE_VALUE)
776778
{
777-
return_last_error:
778-
DWORD error = ::GetLastError();
779-
return error_code(error, system_category());
779+
// Operate on externally provided handle, which must be a directory handle
780+
iterator_handle = params->use_handle;
781+
close_handle = params->close_handle;
780782
}
781-
782-
GetFileInformationByHandleEx_t* get_file_information_by_handle_ex = filesystem::detail::atomic_load_relaxed(get_file_information_by_handle_ex_api);
783-
if (BOOST_LIKELY(get_file_information_by_handle_ex != NULL))
783+
else
784784
{
785-
file_attribute_tag_info info;
786-
BOOL res = get_file_information_by_handle_ex(h.handle, file_attribute_tag_info_class, &info, sizeof(info));
787-
if (BOOST_UNLIKELY(!res))
785+
DWORD flags = FILE_FLAG_BACKUP_SEMANTICS;
786+
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
787+
flags |= FILE_FLAG_OPEN_REPARSE_POINT;
788+
789+
iterator_handle = h.handle = create_file_handle(dir, FILE_LIST_DIRECTORY, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, OPEN_EXISTING, flags);
790+
if (BOOST_UNLIKELY(iterator_handle == INVALID_HANDLE_VALUE))
788791
{
789-
// On FAT/exFAT filesystems requesting FILE_ATTRIBUTE_TAG_INFO returns ERROR_INVALID_PARAMETER. See the comment in symlink_status.
792+
return_last_error:
790793
DWORD error = ::GetLastError();
791-
if (error == ERROR_INVALID_PARAMETER || error == ERROR_NOT_SUPPORTED)
792-
goto use_get_file_information_by_handle;
793-
794794
return error_code(error, system_category());
795795
}
796796

797-
if (BOOST_UNLIKELY((info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0u))
798-
return make_error_code(system::errc::not_a_directory);
799-
800-
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
797+
if (BOOST_LIKELY(get_file_information_by_handle_ex != NULL))
801798
{
802-
if ((info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u && is_reparse_point_tag_a_symlink(info.ReparseTag))
803-
return make_error_code(system::errc::too_many_symbolic_link_levels);
804-
}
805-
}
806-
else
807-
{
808-
use_get_file_information_by_handle:
809-
BY_HANDLE_FILE_INFORMATION info;
810-
BOOL res = ::GetFileInformationByHandle(h.handle, &info);
811-
if (BOOST_UNLIKELY(!res))
812-
goto return_last_error;
799+
file_attribute_tag_info info;
800+
BOOL res = get_file_information_by_handle_ex(iterator_handle, file_attribute_tag_info_class, &info, sizeof(info));
801+
if (BOOST_UNLIKELY(!res))
802+
{
803+
// On FAT/exFAT filesystems requesting FILE_ATTRIBUTE_TAG_INFO returns ERROR_INVALID_PARAMETER. See the comment in symlink_status.
804+
DWORD error = ::GetLastError();
805+
if (error == ERROR_INVALID_PARAMETER || error == ERROR_NOT_SUPPORTED)
806+
goto use_get_file_information_by_handle;
813807

814-
if (BOOST_UNLIKELY((info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0u))
815-
return make_error_code(system::errc::not_a_directory);
808+
return error_code(error, system_category());
809+
}
816810

817-
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
811+
if (BOOST_UNLIKELY((info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0u))
812+
return make_error_code(system::errc::not_a_directory);
813+
814+
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
815+
{
816+
if ((info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u && is_reparse_point_tag_a_symlink(info.ReparseTag))
817+
return make_error_code(system::errc::too_many_symbolic_link_levels);
818+
}
819+
}
820+
else
818821
{
819-
if ((info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u && is_reparse_point_a_symlink_ioctl(h.handle))
820-
return make_error_code(system::errc::too_many_symbolic_link_levels);
822+
use_get_file_information_by_handle:
823+
BY_HANDLE_FILE_INFORMATION info;
824+
BOOL res = ::GetFileInformationByHandle(iterator_handle, &info);
825+
if (BOOST_UNLIKELY(!res))
826+
goto return_last_error;
827+
828+
if (BOOST_UNLIKELY((info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0u))
829+
return make_error_code(system::errc::not_a_directory);
830+
831+
if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u)
832+
{
833+
if ((info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0u && is_reparse_point_a_symlink_ioctl(h.handle))
834+
return make_error_code(system::errc::too_many_symbolic_link_levels);
835+
}
821836
}
822837
}
823838

@@ -826,7 +841,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
826841
{
827842
case file_id_extd_dir_info_format:
828843
{
829-
if (!get_file_information_by_handle_ex(h.handle, file_id_extd_directory_restart_info_class, extra_data, dir_itr_extra_size))
844+
if (!get_file_information_by_handle_ex(iterator_handle, file_id_extd_directory_restart_info_class, extra_data, dir_itr_extra_size))
830845
{
831846
DWORD error = ::GetLastError();
832847

@@ -860,7 +875,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
860875
case file_full_dir_info_format:
861876
fallback_to_file_full_dir_info_format:
862877
{
863-
if (!get_file_information_by_handle_ex(h.handle, file_full_directory_restart_info_class, extra_data, dir_itr_extra_size))
878+
if (!get_file_information_by_handle_ex(iterator_handle, file_full_directory_restart_info_class, extra_data, dir_itr_extra_size))
864879
{
865880
DWORD error = ::GetLastError();
866881

@@ -889,7 +904,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
889904
case file_id_both_dir_info_format:
890905
fallback_to_file_id_both_dir_info:
891906
{
892-
if (!get_file_information_by_handle_ex(h.handle, file_id_both_directory_restart_info_class, extra_data, dir_itr_extra_size))
907+
if (!get_file_information_by_handle_ex(iterator_handle, file_id_both_directory_restart_info_class, extra_data, dir_itr_extra_size))
893908
{
894909
DWORD error = ::GetLastError();
895910

@@ -917,7 +932,7 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
917932
io_status_block iosb;
918933
boost::winapi::NTSTATUS_ status = nt_query_directory_file
919934
(
920-
h.handle,
935+
iterator_handle,
921936
NULL, // Event
922937
NULL, // ApcRoutine
923938
NULL, // ApcContext
@@ -952,8 +967,10 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
952967
break;
953968
}
954969

955-
pimpl->handle = h.handle;
970+
971+
pimpl->handle = iterator_handle;
956972
h.handle = INVALID_HANDLE_VALUE;
973+
pimpl->close_handle = close_handle;
957974

958975
done:
959976
imp.swap(pimpl);
@@ -966,7 +983,8 @@ inline system::error_code dir_itr_close(dir_itr_imp& imp) BOOST_NOEXCEPT
966983
{
967984
if (imp.handle != NULL)
968985
{
969-
::FindClose(imp.handle);
986+
if (BOOST_LIKELY(imp.close_handle))
987+
::FindClose(imp.handle);
970988
imp.handle = NULL;
971989
}
972990

@@ -1020,6 +1038,8 @@ error_code dir_itr_create(boost::intrusive_ptr< detail::dir_itr_imp >& imp, fs::
10201038
return error_code(error, system_category());
10211039
}
10221040

1041+
pimpl->close_handle = true;
1042+
10231043
first_filename = data.cFileName;
10241044
set_file_statuses(data.dwFileAttributes, NULL, first_filename, sf, symlink_sf);
10251045

src/error_handling.hpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,33 @@ typedef boost::winapi::DWORD_ err_t;
8888
#if !defined(STATUS_ACCESS_DENIED)
8989
#define STATUS_ACCESS_DENIED ((boost::winapi::NTSTATUS_)0xC0000022l)
9090
#endif
91+
#if !defined(STATUS_OBJECT_NAME_NOT_FOUND)
92+
#define STATUS_OBJECT_NAME_NOT_FOUND ((boost::winapi::NTSTATUS_)0xC0000034l)
93+
#endif
94+
#if !defined(STATUS_OBJECT_PATH_NOT_FOUND)
95+
#define STATUS_OBJECT_PATH_NOT_FOUND ((boost::winapi::NTSTATUS_)0xC000003Al)
96+
#endif
97+
#if !defined(STATUS_NOT_SUPPORTED)
98+
#define STATUS_NOT_SUPPORTED ((boost::winapi::NTSTATUS_)0xC00000BBl)
99+
#endif
100+
#if !defined(STATUS_BAD_NETWORK_PATH)
101+
#define STATUS_BAD_NETWORK_PATH ((boost::winapi::NTSTATUS_)0xC00000BEl)
102+
#endif
103+
#if !defined(STATUS_DEVICE_DOES_NOT_EXIST)
104+
#define STATUS_DEVICE_DOES_NOT_EXIST ((boost::winapi::NTSTATUS_)0xC00000C0l)
105+
#endif
106+
#if !defined(STATUS_BAD_NETWORK_NAME)
107+
#define STATUS_BAD_NETWORK_NAME ((boost::winapi::NTSTATUS_)0xC00000CCl)
108+
#endif
109+
#if !defined(STATUS_DIRECTORY_NOT_EMPTY)
110+
#define STATUS_DIRECTORY_NOT_EMPTY ((boost::winapi::NTSTATUS_)0xC0000101l)
111+
#endif
112+
#if !defined(STATUS_NOT_A_DIRECTORY)
113+
#define STATUS_NOT_A_DIRECTORY ((boost::winapi::NTSTATUS_)0xC0000103l)
114+
#endif
115+
#if !defined(STATUS_NOT_FOUND)
116+
#define STATUS_NOT_FOUND ((boost::winapi::NTSTATUS_)0xC0000225l)
117+
#endif
91118

92119
//! Converts NTSTATUS error codes to Win32 error codes for reporting
93120
inline boost::winapi::DWORD_ translate_ntstatus(boost::winapi::NTSTATUS_ status)
@@ -106,11 +133,24 @@ inline boost::winapi::DWORD_ translate_ntstatus(boost::winapi::NTSTATUS_ status)
106133
case static_cast< boost::winapi::ULONG_ >(STATUS_NO_MORE_FILES):
107134
return boost::winapi::ERROR_NO_MORE_FILES_;
108135
case static_cast< boost::winapi::ULONG_ >(STATUS_NO_SUCH_DEVICE):
136+
case static_cast< boost::winapi::ULONG_ >(STATUS_DEVICE_DOES_NOT_EXIST):
109137
return boost::winapi::ERROR_DEV_NOT_EXIST_;
110138
case static_cast< boost::winapi::ULONG_ >(STATUS_NO_SUCH_FILE):
139+
case static_cast< boost::winapi::ULONG_ >(STATUS_OBJECT_NAME_NOT_FOUND):
140+
case static_cast< boost::winapi::ULONG_ >(STATUS_OBJECT_PATH_NOT_FOUND):
111141
return boost::winapi::ERROR_FILE_NOT_FOUND_;
112142
case static_cast< boost::winapi::ULONG_ >(STATUS_ACCESS_DENIED):
113143
return boost::winapi::ERROR_ACCESS_DENIED_;
144+
case static_cast< boost::winapi::ULONG_ >(STATUS_BAD_NETWORK_PATH):
145+
return boost::winapi::ERROR_BAD_NETPATH_;
146+
case static_cast< boost::winapi::ULONG_ >(STATUS_BAD_NETWORK_NAME):
147+
return boost::winapi::ERROR_BAD_NET_NAME_;
148+
case static_cast< boost::winapi::ULONG_ >(STATUS_DIRECTORY_NOT_EMPTY):
149+
return boost::winapi::ERROR_DIR_NOT_EMPTY_;
150+
case static_cast< boost::winapi::ULONG_ >(STATUS_NOT_A_DIRECTORY):
151+
return boost::winapi::ERROR_DIRECTORY_; // The directory name is invalid
152+
case static_cast< boost::winapi::ULONG_ >(STATUS_NOT_FOUND):
153+
return boost::winapi::ERROR_NOT_FOUND_;
114154
// map "invalid info class" to "not supported" as this error likely indicates that the kernel does not support what we request
115155
case static_cast< boost::winapi::ULONG_ >(STATUS_INVALID_INFO_CLASS):
116156
default:

0 commit comments

Comments
 (0)