Added protection for CVE-2022-21658 in remove_all on POSIX systems.

Another process could replace the directory being processed by remove_all
with a symlink after remove_all called symlink_status but before
it creates a directory iterator. As a result, remove_all would remove
the linked directory contents instead of removing the symlink.

On POSIX systems that support fdopendir and O_NOFOLLOW flag for open(2),
this can be prevented by opening the directory with O_NOFOLLOW before
iterating. This will fail if the directory was replaced with a symlink.

No protection on other systems.

Reported in https://github.com/boostorg/filesystem/issues/224.

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

build/Jamfile.v2

@@ -112,6 +112,7 @@ project boost/filesystem
       [ check-target-builds ../config//has_stat_st_birthtim "has stat::st_birthtim" : <define>BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIM ]
       [ check-target-builds ../config//has_stat_st_birthtimensec "has stat::st_birthtimensec" : <define>BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIMENSEC ]
       [ check-target-builds ../config//has_stat_st_birthtimespec "has stat::st_birthtimespec" : <define>BOOST_FILESYSTEM_HAS_STAT_ST_BIRTHTIMESPEC ]
+      [ check-target-builds ../config//has_fdopendir_nofollow "has fdopendir(O_NOFOLLOW)" : <define>BOOST_FILESYSTEM_HAS_FDOPENDIR_NOFOLLOW ]
       <conditional>@check-statx
       <conditional>@select-windows-crypto-api
       <conditional>@check-cxx20-atomic-ref

config/Jamfile.v2

@@ -27,6 +27,8 @@ obj has_stat_st_birthtimensec : has_stat_st_birthtimensec.cpp : <include>../src
 explicit has_stat_st_birthtimensec ;
 obj has_stat_st_birthtimespec : has_stat_st_birthtimespec.cpp : <include>../src ;
 explicit has_stat_st_birthtimespec ;
+obj has_fdopendir_nofollow : has_fdopendir_nofollow.cpp : <include>../src ;
+explicit has_fdopendir_nofollow ;
 
 lib bcrypt ;
 explicit bcrypt ;

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 <sys/types.h>
+#include <sys/stat.h>
+#include <dirent.h>
+#include <fcntl.h>
+
+int main()
+{
+    int fd = open(".", O_DIRECTORY | O_RDONLY | O_NOFOLLOW);
+    DIR* dir = fdopendir(fd);
+    return dir != 0;
+}

doc/release_history.html

@@ -42,6 +42,7 @@
 <h2>1.79.0</h2>
 <ul>
   <li>Fixed compilation of path appending and concatenation operators with arguments of types convertible to <code>path</code> or compatible string type. (<a href="https://github.com/boostorg/filesystem/issues/223">#223</a>)</li>
+  <li>On POSIX systems that support <a href="https://pubs.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html"><code>fdopendir</code></a> and <a href="https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html"><code>O_NOFOLLOW</code></a>, <code>remove_all</code> is now protected against <a href="https://www.cve.org/CVERecord?id=CVE-2022-21658">CVE-2022-21658</a>. The vulnerability is a race condition that allows a third party process to replace a directory that is being concurrently processed by <code>remove_all</code> with a directory symlink and cause <code>remove_all</code> to follow the symlink and remove files in the linked directory instead of removing the symlink itself. (<a href="https://github.com/boostorg/filesystem/issues/224">#224</a>)</li>
   <li>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.</li>
 </ul>
 

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)
 

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 <sys/types.h>
+#include <sys/stat.h>
 #include <dirent.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -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 <cwchar>
@@ -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)
         {

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)