chantra / rpms / rpm

Forked from rpms/rpm 2 years ago
Clone
0b2921
From 13f70e3710b2df49a923cc6450ff4a8f86e65666 Mon Sep 17 00:00:00 2001
0b2921
Message-Id: <13f70e3710b2df49a923cc6450ff4a8f86e65666.1555050140.git.pmatilai@redhat.com>
0b2921
From: Panu Matilainen <pmatilai@redhat.com>
0b2921
Date: Wed, 20 Mar 2019 12:38:00 +0200
0b2921
Subject: [PATCH] Fix FA_TOUCH on files with suid/sgid bits and/or capabilities
0b2921
0b2921
FA_TOUCH used to set suffix to "" instead of NULL which causes fsmCommit()
0b2921
to rename the file onto itself, which is a bit dumb but mostly harmless
0b2921
with regular permission. On suid/sgid/capabilities we strip any extra
0b2921
privileges on rename to make sure hardlinks are neutered, and because
0b2921
rename occurs after other permissions etc setting, on FA_TOUCH those
0b2921
extra privileges are stripped and much brokenness will follow.
0b2921
0b2921
A more minimal fix would be a strategically placed strcmp(), but NULL
0b2921
is what the rest of the fsm expects for no suffix and differentiating
0b2921
between empty and NULL suffix is too subtle for its own good as
0b2921
witnessed here. So now, NULL suffix is no suffix again and the rest
0b2921
of the code will do the right thing except where related to creation,
0b2921
and creation is what FA_TOUCH wont do so lets just explicitly skip it
0b2921
and restore the original code otherwise. The goto is ugly but reindenting
0b2921
gets even uglier, shrug. Add a test-case to go with it.
0b2921
0b2921
This has been broken since its introduction in commit
0b2921
79ca74e15e15c1d91a9a31a9ee90abc91736f390 so all current 4.14.x versions
0b2921
are affected.
0b2921
---
0b2921
 lib/fsm.c                         | 17 ++++++++++----
0b2921
 tests/data/SPECS/replacetest.spec |  2 +-
0b2921
 tests/rpmverify.at                | 38 ++++++++++++++++++++++++++++++-
0b2921
 3 files changed, 50 insertions(+), 7 deletions(-)
0b2921
0b2921
diff --git a/lib/fsm.c b/lib/fsm.c
0b2921
index 8eb2c185c..432bcbd90 100644
0b2921
--- a/lib/fsm.c
0b2921
+++ b/lib/fsm.c
0b2921
@@ -898,12 +898,12 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
0b2921
 
0b2921
 	action = rpmfsGetAction(fs, rpmfiFX(fi));
0b2921
 	skip = XFA_SKIPPING(action);
0b2921
-	suffix = S_ISDIR(rpmfiFMode(fi)) ? NULL : tid;
0b2921
 	if (action != FA_TOUCH) {
0b2921
-	    fpath = fsmFsPath(fi, suffix);
0b2921
+	    suffix = S_ISDIR(rpmfiFMode(fi)) ? NULL : tid;
0b2921
 	} else {
0b2921
-	    fpath = fsmFsPath(fi, "");
0b2921
+	    suffix = NULL;
0b2921
 	}
0b2921
+	fpath = fsmFsPath(fi, suffix);
0b2921
 
0b2921
 	/* Remap file perms, owner, and group. */
0b2921
 	rc = rpmfiStat(fi, 1, &sb);
0b2921
@@ -926,6 +926,10 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
0b2921
         if (!skip) {
0b2921
 	    int setmeta = 1;
0b2921
 
0b2921
+	    /* When touching we don't need any of this... */
0b2921
+	    if (action == FA_TOUCH)
0b2921
+		goto touch;
0b2921
+
0b2921
 	    /* Directories replacing something need early backup */
0b2921
 	    if (!suffix) {
0b2921
 		rc = fsmBackup(fi, action);
0b2921
@@ -934,7 +938,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
0b2921
 	    if (!suffix) {
0b2921
 		rc = fsmVerify(fpath, fi);
0b2921
 	    } else {
0b2921
-		rc = (action == FA_TOUCH) ? 0 : RPMERR_ENOENT;
0b2921
+		rc = RPMERR_ENOENT;
0b2921
 	    }
0b2921
 
0b2921
             if (S_ISREG(sb.st_mode)) {
0b2921
@@ -970,11 +974,14 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
0b2921
                 if (!IS_DEV_LOG(fpath))
0b2921
                     rc = RPMERR_UNKNOWN_FILETYPE;
0b2921
             }
0b2921
+
0b2921
+touch:
0b2921
 	    /* Set permissions, timestamps etc for non-hardlink entries */
0b2921
 	    if (!rc && setmeta) {
0b2921
 		rc = fsmSetmeta(fpath, fi, plugins, action, &sb, nofcaps);
0b2921
 	    }
0b2921
         } else if (firsthardlink >= 0 && rpmfiArchiveHasContent(fi)) {
0b2921
+	    /* On FA_TOUCH no hardlinks are created thus this is skipped. */
0b2921
 	    /* we skip the hard linked file containing the content */
0b2921
 	    /* write the content to the first used instead */
0b2921
 	    char *fn = rpmfilesFN(files, firsthardlink);
0b2921
@@ -987,7 +994,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfiles files,
0b2921
         if (rc) {
0b2921
             if (!skip) {
0b2921
                 /* XXX only erase if temp fn w suffix is in use */
0b2921
-                if (suffix && (action != FA_TOUCH)) {
0b2921
+                if (suffix) {
0b2921
 		    (void) fsmRemove(fpath, sb.st_mode);
0b2921
                 }
0b2921
                 errno = saveerrno;
0b2921
diff --git a/tests/data/SPECS/replacetest.spec b/tests/data/SPECS/replacetest.spec
0b2921
index 54974567b..d5a1729d3 100644
0b2921
--- a/tests/data/SPECS/replacetest.spec
0b2921
+++ b/tests/data/SPECS/replacetest.spec
0b2921
@@ -46,4 +46,4 @@ rm -rf $RPM_BUILD_ROOT
0b2921
 
0b2921
 %files
0b2921
 %defattr(-,%{user},%{grp},-)
0b2921
-/opt/*
0b2921
+%{?fileattr} /opt/*
0b2921
diff --git a/tests/rpmverify.at b/tests/rpmverify.at
0b2921
index 52ee2abfb..f7dd57531 100644
0b2921
--- a/tests/rpmverify.at
0b2921
+++ b/tests/rpmverify.at
0b2921
@@ -575,3 +575,39 @@
0b2921
 ],
0b2921
 [])
0b2921
 AT_CLEANUP
0b2921
+
0b2921
+AT_SETUP([Upgraded verification with min_writes 5 (suid files)])
0b2921
+AT_KEYWORDS([upgrade verify min_writes])
0b2921
+AT_CHECK([
0b2921
+RPMDB_CLEAR
0b2921
+RPMDB_INIT
0b2921
+tf="${RPMTEST}"/opt/foo
0b2921
+rm -rf "${tf}" "${tf}".rpm*
0b2921
+rm -rf "${TOPDIR}"
0b2921
+
0b2921
+for v in "1.0" "2.0"; do
0b2921
+    runroot rpmbuild --quiet -bb \
0b2921
+        --define "ver $v" \
0b2921
+	--define "filetype file" \
0b2921
+	--define "filedata foo" \
0b2921
+	--define "fileattr %attr(2755,-,-)" \
0b2921
+          /data/SPECS/replacetest.spec
0b2921
+done
0b2921
+
0b2921
+runroot rpm -U /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
0b2921
+runroot rpm -Va --nouser --nogroup replacetest
0b2921
+runroot rpm -U \
0b2921
+	--define "_minimize_writes 1" \
0b2921
+	 /build/RPMS/noarch/replacetest-2.0-1.noarch.rpm
0b2921
+runroot rpm -Va --nouser --nogroup replacetest
0b2921
+chmod 777 "${tf}"
0b2921
+runroot rpm -U \
0b2921
+	--oldpackage \
0b2921
+	--define "_minimize_writes 1" \
0b2921
+	 /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
0b2921
+runroot rpm -Va --nouser --nogroup replacetest
0b2921
+],
0b2921
+[0],
0b2921
+[],
0b2921
+[])
0b2921
+AT_CLEANUP
0b2921
-- 
0b2921
2.20.1
0b2921