The existing setfscreatecon() at the beginning of copy_file() is the secure
method for setting the context of new files, but it doesn't apply to
existing files. Change the setfilecon() to only run on preexisting files.
v2:
* Use bb_simple_perror_msg() where needed.
* Change setfilecon() to fsetfilecon() to eliminate race between the
dst_file open and file relabel, in case an attacker tries to replace
dst_file so their file can get the setfilecon instead of the intended
file.
* Initialize con variable.
Signed-off-by: Chris PeBenito <chpeb...@linux.microsoft.com>
---
libbb/copy_file.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/libbb/copy_file.c b/libbb/copy_file.c
index 49d1ec9c6..4e5db563f 100644
--- a/libbb/copy_file.c
+++ b/libbb/copy_file.c
@@ -325,19 +325,22 @@ int FAST_FUNC copy_file(const char *source, const char
*dest, int flags)
if ((flags &
(FILEUTILS_PRESERVE_SECURITY_CONTEXT|FILEUTILS_SET_SECURITY_CONTEXT))
&& is_selinux_enabled() > 0
) {
- security_context_t con;
- if (getfscreatecon(&con) == -1) {
+ /* Failure to preserve the security context isn't fatal
here since
+ * the copy has been done at this point. */
+ security_context_t con = NULL;
+ if (getfscreatecon(&con) < 0)
bb_simple_perror_msg("getfscreatecon");
- return -1;
- }
- if (con) {
- if (setfilecon(dest, con) == -1) {
- bb_perror_msg("setfilecon:%s,%s", dest,
con);
- freecon(con);
- return -1;
- }
- freecon(con);
- }
+
+ if (setfscreatecon(NULL) < 0)
+ bb_simple_perror_msg("can't reset fscreate");
+
+ /* setfscreatecon() only works when a file is created.
If dest
+ * preexisted, use fsetfilecon instead */
+ if (con && dest_exists)
+ if (fsetfilecon(dst_fd, con) < 0)
+ bb_perror_msg("fsetfilecon:%s,%s",
dest, con);
+
+ freecon(con);
}
#endif
#if ENABLE_FEATURE_CP_REFLINK
--
2.21.1
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
