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>

src/utils.cpp

@@ -108,6 +108,8 @@ const struct vmlinux_location vmlinux_locs[] = {
   { 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,
@@ -683,6 +685,20 @@ bool is_dir(const std::string &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" }
@@ -721,14 +737,14 @@ std::string unpack_kheaders_tar_xz(const struct utsname &utsname)
 #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();
   }
@@ -749,8 +765,10 @@ 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 "";
   }

src/utils.h

@@ -174,6 +174,7 @@ std::vector<std::string> get_wildcard_tokens(const std::string &input,
 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);

tests/utils.cpp

@@ -358,6 +358,27 @@ TEST(utils, get_pids_for_program)
   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