From 3b269a5e6d0901adc58ce27b9eae51deba679788 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>
Date: Wed, 23 Oct 2019 17:30:19 +0200
Subject: [PATCH] Use and return canonical paths in rpmverifyfile probe

This is done to prevent false positives in implementation of rules
file_permissions_unauthorized_suid and
file_permissions_unauthorized_sgid proposed in ComplianceAsCode
in https://github.com/ComplianceAsCode/content/pull/4648
caused by inconsistent symlink usage in rpmdb.

Also improves logic when an exact file path is defined in
rpmverifyfile_object we don't have to iterate over all RPMs in the
rpmdb but we query the rpmdb directly.
---
 src/OVAL/probes/unix/linux/rpmverifyfile.c | 45 ++++++++++++++++------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/OVAL/probes/unix/linux/rpmverifyfile.c b/src/OVAL/probes/unix/linux/rpmverifyfile.c
index c13e3a629..e17f1612b 100644
--- a/src/OVAL/probes/unix/linux/rpmverifyfile.c
+++ b/src/OVAL/probes/unix/linux/rpmverifyfile.c
@@ -137,6 +137,7 @@ static int rpmverify_collect(probe_ctx *ctx,
 	Header pkgh;
 	pcre *re = NULL;
 	int  ret = -1;
+	char *file_realpath = NULL;
 
 	/* pre-compile regex if needed */
 	if (file_op == OVAL_OPERATION_PATTERN_MATCH) {
@@ -153,7 +154,16 @@ static int rpmverify_collect(probe_ctx *ctx,
 
 	RPMVERIFY_LOCK;
 
-	match = rpmtsInitIterator (g_rpm.rpmts, RPMDBI_PACKAGES, NULL, 0);
+	if (file != NULL && file_op == OVAL_OPERATION_EQUALS) {
+		/*
+		 * When we know the exact file path we look for, we don't need to
+		 * filter all RPM packages, but we can ask the rpmdb directly for
+		 * the package which provides this file, similar to `rpm -q -f`.
+		 */
+		match = rpmtsInitIterator(g_rpm.rpmts, RPMDBI_INSTFILENAMES, file, 0);
+	} else {
+		match = rpmtsInitIterator(g_rpm.rpmts, RPMDBI_PACKAGES, NULL, 0);
+	}
 	if (match == NULL) {
 		ret = 0;
 		goto ret;
@@ -183,6 +193,8 @@ static int rpmverify_collect(probe_ctx *ctx,
 	assume_d(RPMTAG_BASENAMES != 0, -1);
 	assume_d(RPMTAG_DIRNAMES  != 0, -1);
 
+	file_realpath = realpath(file, NULL);
+
 	while ((pkgh = rpmdbNextIterator (match)) != NULL) {
 		SEXP_t *ent;
 		rpmfi  fi;
@@ -190,6 +202,8 @@ static int rpmverify_collect(probe_ctx *ctx,
 		struct rpmverify_res res;
 		errmsg_t rpmerr;
 		int i;
+		const char *current_file;
+		char *current_file_realpath;
 
 		/*
 +SEXP_t *probe_ent_from_cstr(const char *name, oval_datatype_t type,
@@ -231,43 +245,51 @@ static int rpmverify_collect(probe_ctx *ctx,
 		  fi = rpmfiNew(g_rpm.rpmts, pkgh, tag[i], 1);
 
 		  while (rpmfiNext(fi) != -1) {
-				res.file = oscap_strdup(rpmfiFN(fi));
+				current_file = rpmfiFN(fi);
+				current_file_realpath = realpath(current_file, NULL);
 		    res.fflags = rpmfiFFlags(fi);
 		    res.oflags = omit;
 
 		    if (((res.fflags & RPMFILE_CONFIG) && (flags & RPMVERIFY_SKIP_CONFIG)) ||
 					((res.fflags & RPMFILE_GHOST)  && (flags & RPMVERIFY_SKIP_GHOST))) {
-					free(res.file);
+					free(current_file_realpath);
 					continue;
 				}
 
 		    switch(file_op) {
 		    case OVAL_OPERATION_EQUALS:
-					if (strcmp(res.file, file) != 0) {
-						free(res.file);
+					if (strcmp(current_file, file) != 0 &&
+							current_file_realpath && file_realpath &&
+							strcmp(current_file_realpath, file_realpath) != 0) {
+						free(current_file_realpath);
 						continue;
 					}
+					res.file = oscap_strdup(file);
 		      break;
 		    case OVAL_OPERATION_NOT_EQUAL:
-					if (strcmp(res.file, file) == 0) {
-						free(res.file);
+					if (strcmp(current_file, file) == 0 ||
+							(current_file_realpath && file_realpath &&
+							strcmp(current_file_realpath, file_realpath) == 0)) {
+						free(current_file_realpath);
 						continue;
 					}
+					res.file = current_file_realpath ? current_file_realpath : strdup(current_file);
 		      break;
 		    case OVAL_OPERATION_PATTERN_MATCH:
-		      ret = pcre_exec(re, NULL, res.file, strlen(res.file), 0, 0, NULL, 0);
+					ret = pcre_exec(re, NULL, current_file, strlen(current_file), 0, 0, NULL, 0);
 
 		      switch(ret) {
 		      case 0: /* match */
+						res.file = strdup(current_file);
 			break;
 		      case -1:
 			/* mismatch */
-			free(res.file);
+						free(current_file_realpath);
 			continue;
 		      default:
 			dE("pcre_exec() failed!");
 			ret = -1;
-			free(res.file);
+						free(current_file_realpath);
 			goto ret;
 		      }
 		      break;
@@ -275,7 +297,7 @@ static int rpmverify_collect(probe_ctx *ctx,
 		      /* unsupported operation */
 		      dE("Operation \"%d\" on `filepath' not supported", file_op);
 		      ret = -1;
-					free(res.file);
+						free(current_file_realpath);
 		      goto ret;
 		    }
 
@@ -301,6 +323,7 @@ ret:
 		pcre_free(re);
 
 	RPMVERIFY_UNLOCK;
+	free(file_realpath);
 	return (ret);
 }
 
-- 
2.21.0