diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a86783..37ef7e1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,6 +59,7 @@ if(NOT BOOST_FILESYSTEM_DISABLE_STATX) check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_statx_syscall.cpp>" BOOST_FILESYSTEM_HAS_STATX_SYSCALL) endif() endif() +check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_fdopendir_nofollow.cpp>" BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) if(WIN32 AND NOT BOOST_FILESYSTEM_DISABLE_BCRYPT) set(CMAKE_REQUIRED_LIBRARIES bcrypt) check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_bcrypt.cpp>" BOOST_FILESYSTEM_HAS_BCRYPT) @@ -170,6 +171,9 @@ endif() if(BOOST_FILESYSTEM_HAS_STATX_SYSCALL) target_compile_definitions(boost_filesystem PRIVATE BOOST_FILESYSTEM_HAS_STATX_SYSCALL) endif() +if(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) + target_compile_definitions(boost_filesystem PRIVATE BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) +endif() target_link_libraries(boost_filesystem PUBLIC diff --git a/build/Jamfile.v2 b/build/Jamfile.v2 index 6ab58dd..02e3b89 100644 --- a/build/Jamfile.v2 +++ b/build/Jamfile.v2 @@ -112,6 +112,7 @@ project boost/filesystem [ check-target-builds ../config//has_stat_st_birthtim "has stat::st_birthtim" : BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIM ] [ check-target-builds ../config//has_stat_st_birthtimensec "has stat::st_birthtimensec" : BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIMENSEC ] [ check-target-builds ../config//has_stat_st_birthtimespec "has stat::st_birthtimespec" : BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIMESPEC ] + [ check-target-builds ../config//has_fdopendir_nofollow "has fdopendir(O_NOFOLLOW)" : BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW ] @check-statx @select-windows-crypto-api @check-cxx20-atomic-ref diff --git a/config/Jamfile.v2 b/config/Jamfile.v2 index 2e9ee4b..5ea7dc6 100644 --- a/config/Jamfile.v2 +++ b/config/Jamfile.v2 @@ -27,6 +27,8 @@ obj has_stat_st_birthtimensec : has_stat_st_birthtimensec.cpp : ../src explicit has_stat_st_birthtimensec ; obj has_stat_st_birthtimespec : has_stat_st_birthtimespec.cpp : ../src ; explicit has_stat_st_birthtimespec ; +obj has_fdopendir_nofollow : has_fdopendir_nofollow.cpp : ../src ; +explicit has_fdopendir_nofollow ; lib bcrypt ; explicit bcrypt ; diff --git a/config/has_fdopendir_nofollow.cpp b/config/has_fdopendir_nofollow.cpp new file mode 100644 index 0000000..009a730 --- /dev/null +++ b/config/has_fdopendir_nofollow.cpp @@ -0,0 +1,20 @@ +// Copyright 2022 Andrey Semashev + +// Distributed under the Boost Software License, Version 1.0. +// See http://www.boost.org/LICENSE_1_0.txt + +// See library home page at http://www.boost.org/libs/filesystem + +#include "platform_config.hpp" + +#include +#include +#include +#include + +int main() +{ + int fd = open(".", O_DIRECTORY | O_RDONLY | O_NOFOLLOW); + DIR* dir = fdopendir(fd); + return dir != 0; +} diff --git a/doc/release_history.html b/doc/release_history.html index 7403b0f..bd81852 100644 --- a/doc/release_history.html +++ b/doc/release_history.html @@ -42,6 +42,7 @@

1.79.0

  • Fixed compilation of path appending and concatenation operators with arguments of types convertible to path or compatible string type. (#223)
    • +
  • On POSIX systems that support fdopendir and O_NOFOLLOW, remove_all is now protected against CVE-2022-21658. The vulnerability is a race condition that allows a third party process to replace a directory that is being concurrently processed by remove_all with a directory symlink and cause remove_all to follow the symlink and remove files in the linked directory instead of removing the symlink itself. (#224)
  • On Windows, initialize internal WinAPI function pointers early, if possible, to allow Boost.Filesystem operations to be invoked in global constructors. This is only supported on MSVC, GCC, Clang and compatible compilers.
diff --git a/include/boost/filesystem/directory.hpp b/include/boost/filesystem/directory.hpp index f487633..fa1e69b 100644 --- a/include/boost/filesystem/directory.hpp +++ b/include/boost/filesystem/directory.hpp @@ -247,7 +247,8 @@ BOOST_SCOPED_ENUM_UT_DECLARE_BEGIN(directory_options, unsigned int) skip_dangling_symlinks = 1u << 2, // non-standard extension for recursive_directory_iterator: don't follow dangling directory symlinks, pop_on_error = 1u << 3, // non-standard extension for recursive_directory_iterator: instead of producing an end iterator on errors, // repeatedly invoke pop() until it succeeds or the iterator becomes equal to end iterator - _detail_no_push = 1u << 4 // internal use only + _detail_no_follow = 1u << 4, // internal use only + _detail_no_push = 1u << 5 // internal use only } BOOST_SCOPED_ENUM_DECLARE_END(directory_options) diff --git a/src/directory.cpp b/src/directory.cpp index 5542775..8b41ae5 100644 --- a/src/directory.cpp +++ b/src/directory.cpp @@ -2,7 +2,7 @@ // Copyright 2002-2009, 2014 Beman Dawes // Copyright 2001 Dietmar Kuehl -// Copyright 2019 Andrey Semashev +// Copyright 2019, 2022 Andrey Semashev // Distributed under the Boost Software License, Version 1.0. // See http://www.boost.org/LICENSE_1_0.txt @@ -33,6 +33,8 @@ #ifdef BOOST_POSIX_API +#include +#include #include #include #include @@ -47,6 +49,14 @@ #define BOOST_FILESYSTEM_USE_READDIR_R #endif +// At least Mac OS X 10.6 and older doesn't support O_CLOEXEC +#ifndef O_CLOEXEC +#define O_CLOEXEC 0 +#define BOOST_FILESYSTEM_NO_O_CLOEXEC +#endif + +#include "posix_tools.hpp" + #else // BOOST_WINDOWS_API #include @@ -206,13 +216,46 @@ inline std::size_t path_max() #endif // BOOST_FILESYSTEM_USE_READDIR_R -error_code dir_itr_first(void*& handle, void*& buffer, const char* dir, std::string& target, fs::file_status&, fs::file_status&) +error_code dir_itr_first(void*& handle, void*& buffer, const char* dir, std::string& target, unsigned int opts, fs::file_status&, fs::file_status&) { - if ((handle = ::opendir(dir)) == 0) +#if defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) + int flags = O_DIRECTORY | O_RDONLY | O_NDELAY | O_CLOEXEC; + if ((opts & static_cast< unsigned int >(directory_options::_detail_no_follow)) != 0u) + flags |= O_NOFOLLOW; + + int fd = ::open(dir, flags); + if (BOOST_UNLIKELY(fd < 0)) { const int err = errno; return error_code(err, system_category()); } + +#if defined(BOOST_FILESYSTEM_NO_O_CLOEXEC) && defined(FD_CLOEXEC) + int res = ::fcntl(fd, F_SETFD, FD_CLOEXEC); + if (BOOST_UNLIKELY(res < 0)) + { + const int err = errno; + close_fd(fd); + return error_code(err, system_category()); + } +#endif + + handle = ::fdopendir(fd); + if (BOOST_UNLIKELY(!handle)) + { + const int err = errno; + close_fd(fd); + return error_code(err, system_category()); + } +#else // defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) + handle = ::opendir(dir); + if (BOOST_UNLIKELY(!handle)) + { + const int err = errno; + return error_code(err, system_category()); + } +#endif // defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) + target.assign("."); // string was static but caused trouble // when iteration called from dtor, after // static had already been destroyed @@ -342,7 +385,7 @@ error_code dir_itr_increment(void*& handle, void*& buffer, std::string& target, #else // BOOST_WINDOWS_API -error_code dir_itr_first(void*& handle, fs::path const& dir, std::wstring& target, fs::file_status& sf, fs::file_status& symlink_sf) +error_code dir_itr_first(void*& handle, fs::path const& dir, std::wstring& target, unsigned int opts, fs::file_status& sf, fs::file_status& symlink_sf) // Note: an empty root directory has no "." or ".." entries, so this // causes a ERROR_FILE_NOT_FOUND error which we do not considered an // error. It is treated as eof instead. @@ -512,7 +555,7 @@ void directory_iterator_construct(directory_iterator& it, path const& p, unsigne #if defined(BOOST_POSIX_API) imp->buffer, #endif - p.c_str(), filename, file_stat, symlink_file_stat); + p.c_str(), filename, opts, file_stat, symlink_file_stat); if (result) { diff --git a/src/operations.cpp b/src/operations.cpp index e14b598..bf42105 100644 --- a/src/operations.cpp +++ b/src/operations.cpp @@ -945,51 +945,73 @@ inline bool remove_impl(path const& p, error_code* ec) //! remove_all() implementation uintmax_t remove_all_impl(path const& p, error_code* ec) { - fs::file_type type; + for (unsigned int attempt = 0u; attempt < 5u; ++attempt) { - error_code local_ec; - type = fs::detail::symlink_status(p, &local_ec).type(); - - if (type == fs::file_not_found) - return 0u; - - if (BOOST_UNLIKELY(type == fs::status_error)) + fs::file_type type; { - if (!ec) - BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, local_ec)); + error_code local_ec; + type = fs::detail::symlink_status(p, &local_ec).type(); - *ec = local_ec; - return static_cast< uintmax_t >(-1); + if (type == fs::file_not_found) + return 0u; + + if (BOOST_UNLIKELY(type == fs::status_error)) + { + if (!ec) + BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, local_ec)); + + *ec = local_ec; + return static_cast< uintmax_t >(-1); + } } - } - uintmax_t count = 0u; + uintmax_t count = 0u; - if (type == fs::directory_file) // but not a directory symlink - { - fs::directory_iterator itr; - fs::detail::directory_iterator_construct(itr, p, static_cast< unsigned int >(directory_options::none), ec); + if (type == fs::directory_file) // but not a directory symlink + { + fs::directory_iterator itr; + error_code local_ec; + fs::detail::directory_iterator_construct(itr, p, static_cast< unsigned int >(directory_options::_detail_no_follow), &local_ec); + if (BOOST_UNLIKELY(!!local_ec)) + { +#if defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) + // If open(2) with O_NOFOLLOW fails with ELOOP, this means that either the path contains a loop + // of symbolic links, or the last element of the path is a symbolic link. Given that lstat(2) above + // did not fail, most likely it is the latter case. I.e. between the lstat above and this open call + // the filesystem was modified so that the path no longer refers to a directory file (as opposed to a symlink). + if (local_ec == error_code(ELOOP, system_category())) + continue; +#endif // defined(BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW) + + if (!ec) + BOOST_FILESYSTEM_THROW(filesystem_error("boost::filesystem::remove_all", p, local_ec)); + + *ec = local_ec; + return static_cast< uintmax_t >(-1); + } + + const fs::directory_iterator end_dit; + while (itr != end_dit) + { + count += fs::detail::remove_all_impl(itr->path(), ec); + if (ec && *ec) + return static_cast< uintmax_t >(-1); + + fs::detail::directory_iterator_increment(itr, ec); + if (ec && *ec) + return static_cast< uintmax_t >(-1); + } + } + + count += fs::detail::remove_impl(p, type, ec); if (ec && *ec) return static_cast< uintmax_t >(-1); - const fs::directory_iterator end_dit; - while (itr != end_dit) - { - count += fs::detail::remove_all_impl(itr->path(), ec); - if (ec && *ec) - return static_cast< uintmax_t >(-1); - - fs::detail::directory_iterator_increment(itr, ec); - if (ec && *ec) - return static_cast< uintmax_t >(-1); - } + return count; } - count += fs::detail::remove_impl(p, type, ec); - if (ec && *ec) - return static_cast< uintmax_t >(-1); - - return count; + emit_error(ELOOP, p, ec, "boost::filesystem::remove_all: path cannot be opened as a directory"); + return static_cast< uintmax_t >(-1); } #else // defined(BOOST_POSIX_API)