[PATCH] elfcompress: Swap fchmod and fchown calls on new file.

[PATCH] elfcompress: Swap fchmod and fchown calls on new file.

[Mark Wielaard]

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

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog     |  4 ++++
 src/elfcompress.c | 11 +++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index e0f1b51..0e9ab30 100644
--- 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
--- 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)
-- 
1.8.3.1

Re: [PATCH] elfcompress: Swap fchmod and fchown calls on new file.

[Mark Wielaard]

On Sat, Jul 21, 2018 at 05:05:43PM +0200, Igor Gnatenko wrote:
> On Sat, Jul 21, 2018 at 4:56 PM Mark Wielaard <mark@klomp.org> wrote:
> 
> > 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>

Thanks for testing.
Pushed to master.

Cheers,

Mark