CVE-2017-7500: Improper Link Resolution Before File Access ('Link Following') (CWE-59)
value: It was found that rpm did not properly handle RPM installations when a destination path was a symbolic link to a directory, possibly changing ownership and permissions of an arbitrary directory, and RPM files being placed in an arbitrary destination. An attacker, with write access to a directory in which a subdirectory will be installed, could redirect that directory to an arbitrary location and gain root privilege.
CWE description: The product attempts to access a file based on the filename, but it does not properly prevent that filename from identifying a link or shortcut that resolves to an unintended resource.
CVSS v3 base score: 7.8
Published: 2018-08-13T17:29Z
Repository: https://github.com/rpm-software-management/rpm
Language: C
Files changed: lib/verify.c, tests/rpmverify.at
Commit date: 2017-09-28 10:49:24 +0300
Commit message: Make verification match the new restricted directory symlink behavior

Only follow directory symlinks owned by target directory owner or root
during verification to match the behavior of fsmVerify() in the new
CVE-2017-7500 world order.

The code is klunkier than it should and the logic should use common code
with fsmVerify() instead of duplicating it here, but that needs more
changes than is comfortable to backport so starting with this.

Also worth noting that the previous "follow the link" logic from
commit 3ccd774255b8215733e0bdfdf5a683da9dd10923 was not quite right,
it'd fail with RPMVERIFY_LSTATFAIL on a broken symlink when it should've
ran verification on the symlink itself. This behavior is fixed here too.

Finally, once again fakechroot gets in the way and forces the related
verify testcase to be changed to be able to create a valid link. Reuse
the replacement testcase for the purpose and add another case for
verifying an invalid link.

Diff: diff --git a/lib/verify.c b/lib/verify.c
index 0eac812ae..62eec0861 100644
--- a/lib/verify.c
+++ b/lib/verify.c
@@ -92,13 +92,22 @@ rpmVerifyAttrs rpmfilesVerify(rpmfiles fi, int ix, rpmVerifyAttrs omitMask)
 	vfy |= RPMVERIFY_LSTATFAIL;
 	goto exit;
     }
 
     /* If we expected a directory but got a symlink to one, follow the link */
-    if (S_ISDIR(fmode) && S_ISLNK(sb.st_mode) && stat(fn, &sb) != 0) {
-	vfy |= RPMVERIFY_LSTATFAIL;
-	goto exit;
+    if (S_ISDIR(fmode) && S_ISLNK(sb.st_mode)) {
+	struct stat dsb;
+	/* ...if it actually points to a directory  */
+	if (stat(fn, &dsb) == 0 && S_ISDIR(dsb.st_mode)) {
+	    uid_t fuid;
+	    /* ...and is by a legit user, to match fsmVerify() behavior */
+	    if (sb.st_uid == 0 ||
+			(rpmugUid(rpmfilesFUser(fi, ix), &fuid) == 0 &&
+			 sb.st_uid == fuid)) {
+		sb = dsb; /* struct assignment */
+	    }
+	}
     }
 
     /* Links have no mode, other types have no linkto */
     if (S_ISLNK(sb.st_mode))
 	flags &= ~(RPMVERIFY_MODE);
diff --git a/tests/rpmverify.at b/tests/rpmverify.at
index 8e2938259..c5530de69 100644
--- a/tests/rpmverify.at
+++ b/tests/rpmverify.at
@@ -67,22 +67,60 @@ AT_CLEANUP
 AT_SETUP([directory replaced with a directory symlink])
 AT_KEYWORDS([verify])
 AT_CHECK([
 RPMDB_CLEAR
 RPMDB_INIT
+tf="${RPMTEST}"/opt/foo
+rm -rf "${RPMTEST}"/opt/*
+rm -rf "${TOPDIR}"
 
-runroot rpm -U --nodeps --noscripts --ignorearch --ignoreos \
-	/data/RPMS/hello-1.0-1.i386.rpm
-mv "${RPMTEST}"/usr/share/doc/hello-1.0 "${RPMTEST}"/usr/share/doc/hello-1.0.orig
-ln -s hello-1.0.orig "${RPMTEST}"/usr/share/doc/hello-1.0
-runroot rpm -Vva --nodeps --nouser --nogroup
-rm -rf "${RPMTEST}"/usr/share/doc/
+runroot rpmbuild --quiet -bb \
+        --define "ver 1.0" \
+        --define "filetype datadir" \
+        --define "filedata README1" \
+        --define "user $(id -u -n)" \
+          /data/SPECS/replacetest.spec
+
+runroot rpm -U /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
+mv "${RPMTEST}"/opt/foo "${RPMTEST}"/opt/was
+ln -s was "${RPMTEST}"/opt/foo
+runroot rpm -Vv replacetest
 ],
 [0],
-[.........    /usr/local/bin/hello
-.........    /usr/share/doc/hello-1.0
-.........  d /usr/share/doc/hello-1.0/FAQ
+[.........    /opt/foo
+.........    /opt/foo/README1
+.........    /opt/goo
+.........    /opt/zoo
+],
+[])
+AT_CLEANUP
+
+AT_SETUP([directory replaced with an invalid directory symlink])
+AT_KEYWORDS([verify])
+AT_CHECK([
+RPMDB_CLEAR
+RPMDB_INIT
+tf="${RPMTEST}"/opt/foo
+rm -rf "${RPMTEST}"/opt/*
+rm -rf "${TOPDIR}"
+
+runroot rpmbuild --quiet -bb \
+        --define "ver 1.0" \
+        --define "filetype datadir" \
+        --define "filedata README1" \
+          /data/SPECS/replacetest.spec
+
+runroot rpm -U /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
+mv "${RPMTEST}"/opt/foo "${RPMTEST}"/opt/was
+ln -s was "${RPMTEST}"/opt/foo
+runroot rpm -Vv --nouser --nogroup replacetest
+],
+[1],
+[....L....    /opt/foo
+.........    /opt/foo/README1
+.........    /opt/goo
+.........    /opt/zoo
 ],
 [])
 AT_CLEANUP
 
 # Test file verify after mutilating the files a bit.

Vulnerable code:     if (S_ISDIR(fmode) && S_ISLNK(sb.st_mode) && stat(fn, &sb) != 0) {
	vfy |= RPMVERIFY_LSTATFAIL;
	goto exit;
runroot rpm -U --nodeps --noscripts --ignorearch --ignoreos \
	/data/RPMS/hello-1.0-1.i386.rpm
mv "${RPMTEST}"/usr/share/doc/hello-1.0 "${RPMTEST}"/usr/share/doc/hello-1.0.orig
ln -s hello-1.0.orig "${RPMTEST}"/usr/share/doc/hello-1.0
runroot rpm -Vva --nodeps --nouser --nogroup
rm -rf "${RPMTEST}"/usr/share/doc/
[.........    /usr/local/bin/hello
.........    /usr/share/doc/hello-1.0
.........  d /usr/share/doc/hello-1.0/FAQ
Fixed code:     if (S_ISDIR(fmode) && S_ISLNK(sb.st_mode)) {
	struct stat dsb;
	/* ...if it actually points to a directory  */
	if (stat(fn, &dsb) == 0 && S_ISDIR(dsb.st_mode)) {
	    uid_t fuid;
	    /* ...and is by a legit user, to match fsmVerify() behavior */
	    if (sb.st_uid == 0 ||
			(rpmugUid(rpmfilesFUser(fi, ix), &fuid) == 0 &&
			 sb.st_uid == fuid)) {
		sb = dsb; /* struct assignment */
	    }
	}
tf="${RPMTEST}"/opt/foo
rm -rf "${RPMTEST}"/opt/*
rm -rf "${TOPDIR}"
runroot rpmbuild --quiet -bb \
        --define "ver 1.0" \
        --define "filetype datadir" \
        --define "filedata README1" \
        --define "user $(id -u -n)" \
          /data/SPECS/replacetest.spec

runroot rpm -U /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
mv "${RPMTEST}"/opt/foo "${RPMTEST}"/opt/was
ln -s was "${RPMTEST}"/opt/foo
runroot rpm -Vv replacetest
[.........    /opt/foo
.........    /opt/foo/README1
.........    /opt/goo
.........    /opt/zoo
],
[])
AT_CLEANUP

AT_SETUP([directory replaced with an invalid directory symlink])
AT_KEYWORDS([verify])
AT_CHECK([
RPMDB_CLEAR
RPMDB_INIT
tf="${RPMTEST}"/opt/foo
rm -rf "${RPMTEST}"/opt/*
rm -rf "${TOPDIR}"

runroot rpmbuild --quiet -bb \
        --define "ver 1.0" \
        --define "filetype datadir" \
        --define "filedata README1" \
          /data/SPECS/replacetest.spec

runroot rpm -U /build/RPMS/noarch/replacetest-1.0-1.noarch.rpm
mv "${RPMTEST}"/opt/foo "${RPMTEST}"/opt/was
ln -s was "${RPMTEST}"/opt/foo
runroot rpm -Vv --nouser --nogroup replacetest
],
[1],
[....L....    /opt/foo
.........    /opt/foo/README1
.........    /opt/goo
.........    /opt/zoo