arc-setup: Validate media_profiles.xml is not a symlink
Validate /run/arc/oem/etc/media_profiles.xml is not a symlink before
writing to it.
BUG=chromium:1133047
TEST=emerge arc-setup
TEST=cros_workon_make --board=${BOARD} chromeos-base/arc-setup --test
Change-Id: I71d3c91f444d3e601ca6283fe005b3856a12f16c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2486949
Commit-Queue: Ereth McKnight-MacNeil <ereth@chromium.org>
Commit-Queue: Greg Kerr <kerrnel@chromium.org>
Tested-by: Ereth McKnight-MacNeil <ereth@chromium.org>
Tested-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Allen Webb <allenwebb@google.com>
Reviewed-by: Long Cheng <lgcheng@google.com>
diff --git a/arc/setup/arc_setup.cc b/arc/setup/arc_setup.cc
index 1330811..911cbcf 100644
--- a/arc/setup/arc_setup.cc
+++ b/arc/setup/arc_setup.cc
@@ -844,9 +844,15 @@
base::FilePath(oem_mount_directory)
.Append("etc")
.Append(arc_paths_->media_profile_file);
- EXIT_IF(!base::CopyFile(media_profile_xml, new_media_profile_xml));
- EXIT_IF(
- !Chown(kHostArcCameraUid, kHostArcCameraGid, new_media_profile_xml));
+ brillo::SafeFD dest_parent(
+ brillo::SafeFD::Root()
+ .first.OpenExistingDir(new_media_profile_xml.DirName())
+ .first);
+ (void)dest_parent.Unlink(new_media_profile_xml.BaseName().value());
+ EXIT_IF(!SafeCopyFile(
+ media_profile_xml, brillo::SafeFD::Root().first /*src_parent*/,
+ new_media_profile_xml.BaseName(), std::move(dest_parent),
+ 0644 /*permissions*/, kHostArcCameraUid, kHostArcCameraGid));
}
base::FilePath hardware_features_xml("/etc/hardware_features.xml");
@@ -863,7 +869,15 @@
const base::FilePath platform_xml_file =
base::FilePath(oem_mount_directory)
.Append(arc_paths_->platform_xml_file_relative);
- EXIT_IF(!base::CopyFile(hardware_features_xml, platform_xml_file));
+ brillo::SafeFD dest_parent(
+ brillo::SafeFD::Root()
+ .first.OpenExistingDir(platform_xml_file.DirName())
+ .first);
+ (void)dest_parent.Unlink(platform_xml_file.BaseName().value());
+ EXIT_IF(!SafeCopyFile(hardware_features_xml,
+ brillo::SafeFD::Root().first /*src_parent*/,
+ platform_xml_file.BaseName(), std::move(dest_parent),
+ 0644 /*permissions*/));
// TODO(chromium:1083652) Remove dynamic shell scripts once all overlays
// are migrated to static XML config.
diff --git a/arc/setup/arc_setup_util.cc b/arc/setup/arc_setup_util.cc
index 4d10771..605aed8 100644
--- a/arc/setup/arc_setup_util.cc
+++ b/arc/setup/arc_setup_util.cc
@@ -22,6 +22,7 @@
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mount.h>
+#include <sys/sendfile.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/statfs.h>
@@ -1006,4 +1007,56 @@
return true;
}
+bool SafeCopyFile(const base::FilePath& src_path,
+ brillo::SafeFD src_parent,
+ const base::FilePath& dest_path,
+ brillo::SafeFD dest_parent,
+ mode_t permissions,
+ uid_t uid,
+ gid_t gid) {
+ struct stat st;
+ ssize_t len, ret;
+
+ if (!src_parent.is_valid()) {
+ LOG(ERROR) << "Invalid src_parent fd";
+ return false;
+ }
+
+ if (!dest_parent.is_valid()) {
+ LOG(ERROR) << "Invalid dest_parent fd";
+ return false;
+ }
+
+ brillo::SafeFD root = brillo::SafeFD::Root().first;
+ brillo::SafeFD::SafeFDResult result(
+ src_parent.OpenExistingFile(src_path, O_RDONLY | O_CLOEXEC));
+ if (brillo::SafeFD::IsError(result.second)) {
+ LOG(ERROR) << "Failed to open src path " << src_path;
+ return false;
+ }
+ brillo::SafeFD src_fd(std::move(result.first));
+
+ brillo::SafeFD dest_fd(
+ dest_parent.MakeFile(dest_path, permissions, uid, gid).first);
+ if (!dest_fd.is_valid()) {
+ LOG(ERROR) << "Failed to open dest path " << dest_path;
+ return false;
+ }
+
+ fstat(src_fd.get(), &st);
+ len = st.st_size;
+
+ do {
+ ret = sendfile(dest_fd.get(), src_fd.get(), NULL, len);
+ if (ret == -1) {
+ PLOG(ERROR) << "Fail to copy file " << src_path << " to " << dest_path
+ << errno;
+ return false;
+ }
+ len -= ret;
+ } while (len > 0 && ret > 0);
+
+ return true;
+}
+
} // namespace arc
diff --git a/arc/setup/arc_setup_util.h b/arc/setup/arc_setup_util.h
index 0d038dd..966a847 100644
--- a/arc/setup/arc_setup_util.h
+++ b/arc/setup/arc_setup_util.h
@@ -18,6 +18,7 @@
#include <base/files/file_path.h>
#include <base/files/scoped_file.h>
#include <base/macros.h>
+#include <brillo/files/safe_fd.h>
#include "arc/setup/android_sdk_version.h"
@@ -271,6 +272,16 @@
// Returns the user and group ids for a user.
bool GetUserId(const std::string& user, uid_t* user_id, gid_t* group_id);
+// Make a copy of file |src_path| to |dest_path|.
+// Use SafeFD to validate there is no symlink in the path.
+bool SafeCopyFile(const base::FilePath& src_path,
+ brillo::SafeFD src_parent,
+ const base::FilePath& dest_path,
+ brillo::SafeFD dest_parent,
+ mode_t permissions = 0640,
+ uid_t uid = getuid(),
+ gid_t gid = getgid());
+
} // namespace arc
#endif // ARC_SETUP_ARC_SETUP_UTIL_H_
diff --git a/arc/setup/arc_setup_util_test.cc b/arc/setup/arc_setup_util_test.cc
index 9228879..b625d2c 100644
--- a/arc/setup/arc_setup_util_test.cc
+++ b/arc/setup/arc_setup_util_test.cc
@@ -32,6 +32,7 @@
#include <base/time/time.h>
#include <base/timer/elapsed_timer.h>
#include <brillo/file_utils.h>
+#include <brillo/files/safe_fd.h>
#include <gtest/gtest.h>
namespace arc {
@@ -930,4 +931,23 @@
}
}
+TEST(ArcSetupUtil, SafeCopyFile) {
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ const base::FilePath src_file = temp_dir.GetPath().Append("srcfile");
+
+ // Create a new source file and write it.
+ ASSERT_TRUE(WriteToFile(src_file, 0755, "testabc"));
+
+ const base::FilePath dest_file =
+ temp_dir.GetPath().Append("dest").Append("destfile");
+ ASSERT_TRUE(SafeCopyFile(src_file, brillo::SafeFD::Root().first, dest_file,
+ brillo::SafeFD::Root().first));
+
+ const base::FilePath symlink = temp_dir.GetPath().Append("symlink");
+ ASSERT_TRUE(base::CreateSymbolicLink(dest_file, symlink));
+ ASSERT_FALSE(SafeCopyFile(src_file, brillo::SafeFD::Root().first, symlink,
+ brillo::SafeFD::Root().first));
+}
+
} // namespace arc
