From d000375944e8ec965486cf019c3f75f4c06a4e10 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>

Date: Thu, 3 Feb 2022 14:14:21 +0100

Subject: [PATCH 1/5] Prevent file permissions errors

The sysctl setting `fs.protected_regular` doesn't allow `O_CREAT` open

on regular files that we don't own in world writable sticky directories

(think `/tmp`). This causes permission denied error when writing HTML

report to a temporary files created by the `mktemp` command executed as

a normal user and then executing `sudo oscap`.

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

If OpenSCAP fails to open the file because of permissions, it will retry

to open the file without O_CREAT flag.

This fixes only creation of the HTML report using the `--report` option,

We will have to create a similar patch also for other output options

such as `--results` or `--results-arf`.

---

 src/source/xslt.c | 40 ++++++++++++++++++++++++++++------------

 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/source/xslt.c b/src/source/xslt.c

index 24c4c46e9..7a3a3f328 100644

--- a/src/source/xslt.c

+++ b/src/source/xslt.c

@@ -22,6 +22,7 @@

 #include <config.h>

 #endif

+#include <fcntl.h>

 #include <libxml/parser.h>

 #include <libxslt/xslt.h>

 #include <libxslt/xsltInternals.h>

@@ -82,23 +83,38 @@ static int xccdf_ns_xslt_workaround(xmlDocPtr doc, xmlNodePtr node)

 static inline int save_stylesheet_result_to_file(xmlDoc *resulting_doc, xsltStylesheet *stylesheet, const char *outfile)

 {

-	FILE *f = NULL;

-	if (outfile)

-		f = fopen(outfile, "w");

-	else

-		f = stdout;

-

-	if (f == NULL) {

-		oscap_seterr(OSCAP_EFAMILY_OSCAP, "Could not open output file '%s'", outfile ? outfile : "stdout");

-		return -1;

+	int fd = STDOUT_FILENO;

+	if (outfile) {

+#ifdef OS_WINDOWS

+		fd = open(outfile, O_WRONLY|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE);

+#else

+		fd = open(outfile, O_WRONLY|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

+#endif

+		if (fd == -1) {

+			if (errno == EACCES) {

+				/* File already exists and we aren't allowed to create a new one

+				with the same name */

+#ifdef OS_WINDOWS

+				fd = open(outfile, O_WRONLY|O_TRUNC, S_IREAD|S_IWRITE);

+#else

+				fd = open(outfile, O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

+#endif

+			}

+			if (fd == -1) {

+				oscap_seterr(OSCAP_EFAMILY_OSCAP,

+					"Could not open output file '%s': %s",

+					outfile, strerror(errno));

+				return -1;

+			}

+		}

 	}

-	int ret = xsltSaveResultToFile(f, resulting_doc, stylesheet);

+	int ret = xsltSaveResultToFd(fd, resulting_doc, stylesheet);

 	if (ret < 0) {

 		oscap_seterr(OSCAP_EFAMILY_OSCAP, "Could not save result document");

 	}

-	if (outfile && f)

-		fclose(f);

+	if (fd != STDOUT_FILENO)

+		close(fd);

 	return ret;

 }

From ad3c89a72c0aeb6c6ceab0873c51b07deba45701 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>

Date: Thu, 3 Feb 2022 15:08:52 +0100

Subject: [PATCH 2/5] Prevent permission access issues

Very similar to the previous commit, but this time for saving XML

documents.

---

 src/common/elements.c | 19 ++++++++++++++++---

 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/common/elements.c b/src/common/elements.c

index e512f5e3d..d0d9170f1 100644

--- a/src/common/elements.c

+++ b/src/common/elements.c

@@ -233,9 +233,22 @@ int oscap_xml_save_filename(const char *filename, xmlDocPtr doc)

 		int fd = open(filename, O_CREAT|O_TRUNC|O_WRONLY,

 				S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

 #endif

-		if (fd < 0) {

-			oscap_seterr(OSCAP_EFAMILY_GLIBC, "%s '%s'", strerror(errno), filename);

-			return -1;

+		if (fd == -1) {

+			if (errno == EACCES) {

+				/* File already exists and we aren't allowed to create a new one

+				with the same name */

+#ifdef OS_WINDOWS

+				fd = open(filename, O_WRONLY|O_TRUNC, S_IREAD|S_IWRITE);

+#else

+				fd = open(filename, O_WRONLY|O_TRUNC,

+						S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

+#endif

+			}

+			if (fd == -1) {

+				oscap_seterr(OSCAP_EFAMILY_GLIBC,

+						"%s '%s'", strerror(errno), filename);

+				return -1;

+			}

 		}

 		buff = xmlOutputBufferCreateFd(fd, NULL);

From b2dc90fb80419e30d05676660a2069050693078d Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>

Date: Thu, 3 Feb 2022 15:37:31 +0100

Subject: [PATCH 3/5] Add a missing include

---

 src/source/xslt.c | 2 ++

 1 file changed, 2 insertions(+)

diff --git a/src/source/xslt.c b/src/source/xslt.c

index 7a3a3f328..a763d6b59 100644

--- a/src/source/xslt.c

+++ b/src/source/xslt.c

@@ -30,6 +30,8 @@

 #include <libxslt/xsltutils.h>

 #include <libexslt/exslt.h>

 #include <string.h>

+#include <sys/stat.h>

+

 #ifdef OS_WINDOWS

 #include <io.h>

 #else

From 13ff98bd744ea542bc782e388fdedb5b7f66e54b Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>

Date: Thu, 3 Feb 2022 16:22:56 +0100

Subject: [PATCH 4/5] Fix missing STDOUT_FILENO on Windows

---

 src/source/xslt.c | 8 ++++++++

 1 file changed, 8 insertions(+)

diff --git a/src/source/xslt.c b/src/source/xslt.c

index a763d6b59..799d5170d 100644

--- a/src/source/xslt.c

+++ b/src/source/xslt.c

@@ -85,7 +85,11 @@ static int xccdf_ns_xslt_workaround(xmlDocPtr doc, xmlNodePtr node)

 static inline int save_stylesheet_result_to_file(xmlDoc *resulting_doc, xsltStylesheet *stylesheet, const char *outfile)

 {

+#ifdef OS_WINDOWS

+	int fd = _fileno(stdout);

+#else

 	int fd = STDOUT_FILENO;

+#endif

 	if (outfile) {

 #ifdef OS_WINDOWS

 		fd = open(outfile, O_WRONLY|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE);

@@ -115,7 +119,11 @@ static inline int save_stylesheet_result_to_file(xmlDoc *resulting_doc, xsltStyl

 	if (ret < 0) {

 		oscap_seterr(OSCAP_EFAMILY_OSCAP, "Could not save result document");

 	}

+#ifdef OS_WINDOWS

+	if (fd != _fileno(stdout))

+#else

 	if (fd != STDOUT_FILENO)

+#endif

 		close(fd);

 	return ret;

 }

From f00831513ec2cdde13c12820ff6cc0eef8105c65 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>

Date: Fri, 4 Feb 2022 12:19:50 +0100

Subject: [PATCH 5/5] Refactor: extract function oscap_open_writable

---

 src/common/elements.c                         | 26 ++--------------

 src/common/util.c                             | 31 +++++++++++++++++++

 src/common/util.h                             | 14 ++++++++-

 src/source/xslt.c                             | 27 ++--------------

 tests/API/XCCDF/unittests/CMakeLists.txt      |  2 ++

 tests/API/XCCDF/unittests/test_oscap_common.c |  1 +

 6 files changed, 53 insertions(+), 48 deletions(-)

diff --git a/src/common/elements.c b/src/common/elements.c

index d0d9170f1..de4be88f1 100644

--- a/src/common/elements.c

+++ b/src/common/elements.c

@@ -227,29 +227,9 @@ int oscap_xml_save_filename(const char *filename, xmlDocPtr doc)

 		xmlCode = xmlSaveFormatFileEnc(filename, doc, "UTF-8", 1);

 	}

 	else {

-#ifdef OS_WINDOWS

-		int fd = open(filename, O_CREAT|O_TRUNC|O_WRONLY, S_IREAD|S_IWRITE);

-#else

-		int fd = open(filename, O_CREAT|O_TRUNC|O_WRONLY,

-				S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

-#endif

-		if (fd == -1) {

-			if (errno == EACCES) {

-				/* File already exists and we aren't allowed to create a new one

-				with the same name */

-#ifdef OS_WINDOWS

-				fd = open(filename, O_WRONLY|O_TRUNC, S_IREAD|S_IWRITE);

-#else

-				fd = open(filename, O_WRONLY|O_TRUNC,

-						S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

-#endif

-			}

-			if (fd == -1) {

-				oscap_seterr(OSCAP_EFAMILY_GLIBC,

-						"%s '%s'", strerror(errno), filename);

-				return -1;

-			}

-		}

+		int fd = oscap_open_writable(filename);

+		if (fd == -1)

+			return -1;

 		buff = xmlOutputBufferCreateFd(fd, NULL);

 		if (buff == NULL) {

diff --git a/src/common/util.c b/src/common/util.c

index 8f9f751e2..8ca2ad130 100644

--- a/src/common/util.c

+++ b/src/common/util.c

@@ -25,12 +25,14 @@

 #include <config.h>

 #endif

+#include <fcntl.h>

 #include <string.h>

 #include <ctype.h>

 #include <limits.h>

 #include <stdarg.h>

 #include <math.h>

 #include <pcre.h>

+#include <sys/stat.h>

 #include "util.h"

 #include "_error.h"

@@ -482,3 +484,32 @@ char *oscap_windows_error_message(unsigned long error_code)

 	return error_message;

 }

 #endif

+

+int oscap_open_writable(const char *filename)

+{

+#ifdef OS_WINDOWS

+	int fd = open(filename, O_WRONLY|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE);

+#else

+	int fd = open(filename, O_WRONLY|O_CREAT|O_TRUNC,

+			S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

+#endif

+	if (fd == -1) {

+		if (errno == EACCES) {

+			/* File already exists and we aren't allowed to create a new one

+			with the same name */

+#ifdef OS_WINDOWS

+			fd = open(filename, O_WRONLY|O_TRUNC, S_IREAD|S_IWRITE);

+#else

+			fd = open(filename, O_WRONLY|O_TRUNC,

+					S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

+#endif

+		}

+		if (fd == -1) {

+			oscap_seterr(OSCAP_EFAMILY_OSCAP,

+					"Could not open output file '%s': %s",

+					filename, strerror(errno));

+			return -1;

+		}

+	}

+	return fd;

+}

diff --git a/src/common/util.h b/src/common/util.h

index c48d92a52..5712f2f0b 100644

--- a/src/common/util.h

+++ b/src/common/util.h

@@ -524,4 +524,16 @@ wchar_t *oscap_windows_str_to_wstr(const char *str);

 char *oscap_windows_error_message(unsigned long error_code);

 #endif

-#endif				/* OSCAP_UTIL_H_ */

+/**

+ * Open a file for writing.

+ * The main difference from fopen() is that if the file exists but its opening

+ * for writing fails as permission denied, it will attempt to open it again

+ * without the O_CREAT flag. This is useful when writing to world-writeable

+ * directories with sticky bit such as /tmp on systems with fs.protected_regular

+ * turned on.

+ * @param filename name of the file to be opened

+ * @return file descriptor or -1 on error

+ */

+int oscap_open_writable(const char *filename);

+

+#endif              /* OSCAP_UTIL_H_ */

diff --git a/src/source/xslt.c b/src/source/xslt.c

index 799d5170d..906bfabdb 100644

--- a/src/source/xslt.c

+++ b/src/source/xslt.c

@@ -22,7 +22,6 @@

 #include <config.h>

 #endif

-#include <fcntl.h>

 #include <libxml/parser.h>

 #include <libxslt/xslt.h>

 #include <libxslt/xsltInternals.h>

@@ -30,7 +29,6 @@

 #include <libxslt/xsltutils.h>

 #include <libexslt/exslt.h>

 #include <string.h>

-#include <sys/stat.h>

 #ifdef OS_WINDOWS

 #include <io.h>

@@ -91,29 +89,10 @@ static inline int save_stylesheet_result_to_file(xmlDoc *resulting_doc, xsltStyl

 	int fd = STDOUT_FILENO;

 #endif

 	if (outfile) {

-#ifdef OS_WINDOWS

-		fd = open(outfile, O_WRONLY|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE);

-#else

-		fd = open(outfile, O_WRONLY|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

-#endif

-		if (fd == -1) {

-			if (errno == EACCES) {

-				/* File already exists and we aren't allowed to create a new one

-				with the same name */

-#ifdef OS_WINDOWS

-				fd = open(outfile, O_WRONLY|O_TRUNC, S_IREAD|S_IWRITE);

-#else

-				fd = open(outfile, O_WRONLY|O_TRUNC, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH);

-#endif

-			}

-			if (fd == -1) {

-				oscap_seterr(OSCAP_EFAMILY_OSCAP,

-					"Could not open output file '%s': %s",

-					outfile, strerror(errno));

-				return -1;

-			}

-		}

+		fd = oscap_open_writable(outfile);

 	}

+	if (fd == -1)

+		return -1;

 	int ret = xsltSaveResultToFd(fd, resulting_doc, stylesheet);

 	if (ret < 0) {

diff --git a/tests/API/XCCDF/unittests/CMakeLists.txt b/tests/API/XCCDF/unittests/CMakeLists.txt

index 97e1ab09f..a1c7da67f 100644

--- a/tests/API/XCCDF/unittests/CMakeLists.txt

+++ b/tests/API/XCCDF/unittests/CMakeLists.txt

@@ -2,6 +2,8 @@ add_oscap_test_executable(test_oscap_common

 	"test_oscap_common.c"

 	${CMAKE_SOURCE_DIR}/src/common/util.c

 	${CMAKE_SOURCE_DIR}/src/common/list.c

+	${CMAKE_SOURCE_DIR}/src/common/error.c

+	${CMAKE_SOURCE_DIR}/src/common/err_queue.c

 )

 add_oscap_test_executable(test_xccdf_overrides

diff --git a/tests/API/XCCDF/unittests/test_oscap_common.c b/tests/API/XCCDF/unittests/test_oscap_common.c

index 26ff51d48..939ba8750 100644

--- a/tests/API/XCCDF/unittests/test_oscap_common.c

+++ b/tests/API/XCCDF/unittests/test_oscap_common.c

@@ -28,6 +28,7 @@

 #include <string.h>

 #include "common/list.h"

 #include "common/util.h"

+#include "common/_error.h"

 #include "oscap_assert.h"

 #define SEEN_LEN 9