elfcompress: Swap fchmod and fchown calls on new file.

Calling fchmod with a suid bit on a file might silently fail or the suid
bit might be slilently cleared by a call to fchown if already set. Swap
the calls so that the owner is set first and then set the suid bit.

https://bugzilla.redhat.com/show_bug.cgi?id=1607044

Reported-and-tested-by: Igor Gnatenko <ignatenkobrain@fedoraproject.org>
Signed-off-by: Mark Wielaard <mark@klomp.org>

diff --git a/src/ChangeLog b/src/ChangeLog
index e0f1b51..0e9ab30 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2018-07-21  Mark Wielaard  <mark@klomp.org>
+
+       * elfcompress.c (process_file): Swap fchmod and fchown calls.
+
 2018-07-04  Mark Wielaard  <mark@klomp.org>
 
        * readelf.c (print_debug_addr_section): Rename index var to uidx.

diff --git a/src/elfcompress.c b/src/elfcompress.c
index bdb0e3b..1a0f984 100644 (file)
--- a/src/elfcompress.c
+++ b/src/elfcompress.c
@@ -1235,13 +1235,16 @@ process_file (const char *fname)
   elf_end (elfnew);
   elfnew = NULL;
 
-  /* Try to match mode and owner.group of the original file.  */
-  if (fchmod (fdnew, st.st_mode & ALLPERMS) != 0)
-    if (verbose >= 0)
-      error (0, errno, "Couldn't fchmod %s", fnew);
+  /* Try to match mode and owner.group of the original file.
+     Note to set suid bits we have to make sure the owner is setup
+     correctly first. Otherwise fchmod will drop them silently
+     or fchown may clear them.  */
   if (fchown (fdnew, st.st_uid, st.st_gid) != 0)
     if (verbose >= 0)
       error (0, errno, "Couldn't fchown %s", fnew);
+  if (fchmod (fdnew, st.st_mode & ALLPERMS) != 0)
+    if (verbose >= 0)
+      error (0, errno, "Couldn't fchmod %s", fnew);
 
   /* Finally replace the old file with the new file.  */
   if (foutput == NULL)