From 60bc072bc317fda90a19d2d8ae621051541277aa Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Thu, 8 Jul 2021 06:43:04 +0000
Subject: [PATCH] util/system: Close non-std fds when execing slave processes

---
 configure.ac               | 16 +++++++++++-
 src/common/run_command.cpp |  3 +++
 src/common/run_command.h   | 50 ++++++++++++++++++++++++++++++++++++++
 test/lint/lint-includes.py |  1 +
 4 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index b5116922f9..f93671a467 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1505,11 +1505,25 @@ if test "$use_external_signer" != "no"; then
   AC_LINK_IFELSE([AC_LANG_PROGRAM([[
     #define BOOST_PROCESS_USE_STD_FS
     #include <boost/process.hpp>
+
+    #ifdef BOOST_POSIX_API
+    # include <fcntl.h>
+    # ifdef FD_CLOEXEC
+    #  include <boost/process/extend.hpp>
+    # endif
+    #endif
   ]],[[
     namespace bp = boost::process;
     bp::opstream stdin_stream;
     bp::ipstream stdout_stream;
-    bp::child c("dummy", bp::std_out > stdout_stream, bp::std_err > stdout_stream, bp::std_in < stdin_stream);
+    #if defined(BOOST_POSIX_API) && defined(FD_CLOEXEC)
+    struct dummy_extension : boost::process::extend::handler {};
+    #endif
+    bp::child c("dummy", bp::std_out > stdout_stream, bp::std_err > stdout_stream, bp::std_in < stdin_stream
+    #if defined(BOOST_POSIX_API) && defined(FD_CLOEXEC)
+        , dummy_extension()
+    #endif
+    );
     stdin_stream << std::string{"test"} << std::endl;
     if (c.running()) c.terminate();
     c.wait();
diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp
index 6ad9f75b5d..06cac84d65 100644
--- a/src/common/run_command.cpp
+++ b/src/common/run_command.cpp
@@ -41,6 +41,9 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string&
         bp::std_out > stdout_stream,
         bp::std_err > stderr_stream,
         bp::std_in < stdin_stream
+#ifdef HAVE_BPE_CLOSE_EXCESS_FDS
+        , bpe_close_excess_fds()
+#endif
     );
     if (!str_std_in.empty()) {
         stdin_stream << str_std_in << std::endl;
diff --git a/src/common/run_command.h b/src/common/run_command.h
index 2fbdc071ee..2a52649bbf 100644
--- a/src/common/run_command.h
+++ b/src/common/run_command.h
@@ -5,10 +5,60 @@
 #ifndef BITCOIN_COMMON_RUN_COMMAND_H
 #define BITCOIN_COMMON_RUN_COMMAND_H
 
+#if defined(HAVE_CONFIG_H)
+#include <config/bitcoin-config.h>
+#endif
+
+#include <util/fs.h>
+#include <util/strencodings.h>
+
 #include <string>
 
+#if defined(ENABLE_EXTERNAL_SIGNER) && defined(BOOST_POSIX_API)
+#include <fcntl.h>
+#ifdef FD_CLOEXEC
+#include <unistd.h>
+#if defined(__GNUC__)
+// Boost 1.78 requires the following workaround.
+// See: https://github.com/boostorg/process/issues/235
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnarrowing"
+#endif
+#include <boost/process.hpp>
+#include <boost/process/extend.hpp>
+#if defined(__GNUC__)
+#pragma GCC diagnostic pop
+#endif
+#endif // FD_CLOEXEC
+#endif // ENABLE_EXTERNAL_SIGNER && BOOST_POSIX_API
+
 class UniValue;
 
+#if defined(ENABLE_EXTERNAL_SIGNER) && defined(BOOST_POSIX_API) && defined(FD_CLOEXEC)
+/**
+ * Ensure a boost::process::child has its non-std fds all closed when exec
+ * is called.
+ */
+struct bpe_close_excess_fds : boost::process::extend::handler
+{
+    template<typename Executor>
+    void on_exec_setup(Executor&exec) const
+    {
+        try {
+            for (auto it : fs::directory_iterator("/dev/fd")) {
+                int64_t fd;
+                if (!ParseInt64(it.path().filename().native(), &fd)) continue;
+                if (fd <= 2) continue;  // leave std{in,out,err} alone
+                ::fcntl(fd, F_SETFD, ::fcntl(fd, F_GETFD) | FD_CLOEXEC);
+            }
+        } catch (...) {
+            // TODO: maybe log this - but we're in a child process, so maybe non-trivial!
+        }
+    }
+};
+#define HAVE_BPE_CLOSE_EXCESS_FDS
+#endif
+
 /**
  * Execute a command which returns JSON, and parse the result.
  *
diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py
index 459030bb0b..dca149e519 100755
--- a/test/lint/lint-includes.py
+++ b/test/lint/lint-includes.py
@@ -27,6 +27,7 @@ EXPECTED_BOOST_INCLUDES = ["boost/date_time/posix_time/posix_time.hpp",
                            "boost/multi_index/sequenced_index.hpp",
                            "boost/multi_index_container.hpp",
                            "boost/process.hpp",
+                           "boost/process/extend.hpp",
                            "boost/signals2/connection.hpp",
                            "boost/signals2/optional_last_value.hpp",
                            "boost/signals2/signal.hpp",