CVE-2024-2313: Insufficient Information (NVD-CWE-noinfo)
value: If kernel headers need to be extracted, bpftrace will attempt to load them from a temporary directory. An unprivileged attacker could use this to force bcc to load compromised linux headers. Linux distributions which provide kernel headers by default are not affected by default.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-10T23:15Z
Repository: https://github.com/bpftrace/bpftrace
Language: C++
Files changed: src/utils.cpp, src/utils.h, tests/utils.cpp
Commit date: 2024-03-06 13:59:05 -0500
Commit message: Fix security hole checking unpacked kernel headers (#3033)

Make sure to check that the unpacked kheaders tar
is owned by root to prevent bpftrace from loading
compromised linux headers.

Co-authored-by: Jordan Rome <jordalgo@fedoraproject.org>
Diff: diff --git a/src/utils.cpp b/src/utils.cpp
index 0a3af640..e9f412e4 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -106,10 +106,12 @@ const struct vmlinux_location vmlinux_locs[] = {
   { "/usr/lib/debug/boot/vmlinux-%1$s.debug", false },
   { "/usr/lib/debug/lib/modules/%1$s/vmlinux", false },
   { nullptr, false },
 };
 
+constexpr std::string_view PROC_KHEADERS_PATH = "/sys/kernel/kheaders.tar.xz";
+
 static bool pid_in_different_mountns(int pid);
 static std::vector<std::string> resolve_binary_path(const std::string &cmd,
                                                     const char *env_paths,
                                                     int pid);
 
@@ -681,10 +683,24 @@ bool is_dir(const std::string &path)
   std::error_code ec;
   std_filesystem::path buf{ path };
   return std_filesystem::is_directory(buf, ec);
 }
 
+bool file_exists_and_ownedby_root(const char *f)
+{
+  struct stat st;
+  if (stat(f, &st) == 0) {
+    if (st.st_uid != 0) {
+      LOG(ERROR) << "header file ownership expected to be root: "
+                 << std::string(f);
+      return false;
+    }
+    return true;
+  }
+  return false;
+}
+
 namespace {
 struct KernelHeaderTmpDir {
   KernelHeaderTmpDir(const std::string &prefix) : path{ prefix + "XXXXXX" }
   {
     if (::mkdtemp(&path[0]) == nullptr) {
@@ -719,18 +735,18 @@ std::string unpack_kheaders_tar_xz(const struct utsname &utsname)
 #if defined(__ANDROID__)
   std_filesystem::path path_prefix{ "/data/local/tmp" };
 #else
   std_filesystem::path path_prefix{ "/tmp" };
 #endif
-  std_filesystem::path path_kheaders{ "/sys/kernel/kheaders.tar.xz" };
+  std_filesystem::path path_kheaders{ PROC_KHEADERS_PATH };
   if (const char *tmpdir = ::getenv("TMPDIR")) {
     path_prefix = tmpdir;
   }
   path_prefix /= "kheaders-";
   std_filesystem::path shared_path{ path_prefix.string() + utsname.release };
 
-  if (std_filesystem::exists(shared_path, ec)) {
+  if (file_exists_and_ownedby_root(shared_path.c_str())) {
     // already unpacked
     return shared_path.string();
   }
 
   if (!std_filesystem::exists(path_kheaders, ec)) {
@@ -747,12 +763,14 @@ std::string unpack_kheaders_tar_xz(const struct utsname &utsname)
     }
   }
 
   KernelHeaderTmpDir tmpdir{ path_prefix };
 
-  FILE *tar = ::popen(
-      ("tar xf /sys/kernel/kheaders.tar.xz -C " + tmpdir.path).c_str(), "w");
+  FILE *tar = ::popen(("tar xf " + std::string(PROC_KHEADERS_PATH) + " -C " +
+                       tmpdir.path)
+                          .c_str(),
+                      "w");
   if (!tar) {
     return "";
   }
 
   int rc = ::pclose(tar);
diff --git a/src/utils.h b/src/utils.h
index c6f78495..e048b037 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -172,10 +172,11 @@ std::vector<std::string> get_wildcard_tokens(const std::string &input,
                                              bool &start_wildcard,
                                              bool &end_wildcard);
 std::vector<int> get_online_cpus();
 std::vector<int> get_possible_cpus();
 bool is_dir(const std::string &path);
+bool file_exists_and_ownedby_root(const char *f);
 std::tuple<std::string, std::string> get_kernel_dirs(
     const struct utsname &utsname,
     bool unpack_kheaders = true);
 std::vector<std::string> get_kernel_cflags(const char *uname_machine,
                                            const std::string &ksrc,
diff --git a/tests/utils.cpp b/tests/utils.cpp
index d5f8a2d7..e996f5a4 100644
--- a/tests/utils.cpp
+++ b/tests/utils.cpp
@@ -356,8 +356,29 @@ TEST(utils, get_pids_for_program)
 
   pids = get_pids_for_program("/proc/12345/root/usr/bin/bash");
   ASSERT_EQ(pids.size(), 0);
 }
 
+TEST(utils, file_exists_and_ownedby_root)
+{
+  std::string tmpdir = "/tmp/bpftrace-test-utils-XXXXXX";
+  std::string file1 = "/ownedby-user";
+  std::string file2 = "/no-exists";
+  if (::mkdtemp(tmpdir.data()) == nullptr) {
+    throw std::runtime_error("creating temporary path for tests failed");
+  }
+
+  int fd;
+  fd = open((tmpdir + file1).c_str(), O_CREAT, S_IRUSR);
+  close(fd);
+  ASSERT_GE(fd, 0);
+
+  EXPECT_FALSE(file_exists_and_ownedby_root((tmpdir + file1).c_str()));
+  EXPECT_FALSE(file_exists_and_ownedby_root((tmpdir + file2).c_str()));
+  EXPECT_TRUE(file_exists_and_ownedby_root("/proc/1/maps"));
+
+  EXPECT_GT(std_filesystem::remove_all(tmpdir), 0);
+}
+
 } // namespace utils
 } // namespace test
 } // namespace bpftrace

Vulnerable code:   std_filesystem::path path_kheaders{ "/sys/kernel/kheaders.tar.xz" };
  if (std_filesystem::exists(shared_path, ec)) {
  FILE *tar = ::popen(
      ("tar xf /sys/kernel/kheaders.tar.xz -C " + tmpdir.path).c_str(), "w");
Fixed code: constexpr std::string_view PROC_KHEADERS_PATH = "/sys/kernel/kheaders.tar.xz";

bool file_exists_and_ownedby_root(const char *f)
{
  struct stat st;
  if (stat(f, &st) == 0) {
    if (st.st_uid != 0) {
      LOG(ERROR) << "header file ownership expected to be root: "
                 << std::string(f);
      return false;
    }
    return true;
  }
  return false;
}

  std_filesystem::path path_kheaders{ PROC_KHEADERS_PATH };
  if (file_exists_and_ownedby_root(shared_path.c_str())) {
  FILE *tar = ::popen(("tar xf " + std::string(PROC_KHEADERS_PATH) + " -C " +
                       tmpdir.path)
                          .c_str(),
                      "w");
bool file_exists_and_ownedby_root(const char *f);
TEST(utils, file_exists_and_ownedby_root)
{
  std::string tmpdir = "/tmp/bpftrace-test-utils-XXXXXX";
  std::string file1 = "/ownedby-user";
  std::string file2 = "/no-exists";
  if (::mkdtemp(tmpdir.data()) == nullptr) {
    throw std::runtime_error("creating temporary path for tests failed");
  }

  int fd;
  fd = open((tmpdir + file1).c_str(), O_CREAT, S_IRUSR);
  close(fd);
  ASSERT_GE(fd, 0);

  EXPECT_FALSE(file_exists_and_ownedby_root((tmpdir + file1).c_str()));
  EXPECT_FALSE(file_exists_and_ownedby_root((tmpdir + file2).c_str()));
  EXPECT_TRUE(file_exists_and_ownedby_root("/proc/1/maps"));

  EXPECT_GT(std_filesystem::remove_all(tmpdir), 0);
}