From 454a9f248ba8f0959a85bf917a7b5b87ab71d1e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgzones@googlemail.com>
Date: Tue, 19 Dec 2023 17:09:32 +0100
Subject: [PATCH] libselinux: enable usage with pedantic UB sanitizers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Clang's undefined behavior sanitizer supports checking for unsigned
integer overflow and underflow, and implicit conversions.  While those
operations are well-defined by the C language they can signal logic
mistakes or processing of unchecked user input.

Annotate functions deliberately making use of integer overflow and adopt
the remaining code sites.

Example reports:

    stringrep.c:348:7: runtime error: left shift of 2147483648 by 1 places cannot be represented in type 'access_vector_t' (aka 'unsigned int')
    seusers.c:98:14: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'gid_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: James Carter <jwcart2@gmail.com>

Reference:https://github.com/SELinuxProject/selinux/commit/454a9f248ba8f0959a85bf917a7b5b87ab71d1e7
Conflict:path and context adapt, label_file.c: del symhash modification

---
 src/avc.c                    |  4 +++-
 src/avc_sidtab.c             |  1 +
 src/label.c                  |  7 +++++--
 src/label_backends_android.c |  4 +++-
 src/label_db.c               |  3 ++-
 src/label_file.c             |  6 ++++--
 src/label_media.c            |  4 +++-
 src/label_x.c                |  4 +++-
 src/selinux_internal.h       | 11 +++++++++++
 src/seusers.c                |  2 +-
 src/sha1.c                   |  3 +++
 src/stringrep.c              |  4 +++-
 12 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/src/avc.c b/src/avc.c
index 5e1c036e..ce87ac16 100644
--- a/src/avc.c
+++ b/src/avc.c
@@ -229,13 +229,15 @@ int avc_open(struct selinux_opt *opts, unsigned nopts)
 {
 	avc_setenforce = 0;
 
-	while (nopts--)
+	while (nopts) {
+		nopts--;
 		switch(opts[nopts].type) {
 		case AVC_OPT_SETENFORCE:
 			avc_setenforce = 1;
 			avc_enforcing = !!opts[nopts].value;
 			break;
 		}
+	}
 
 	return avc_init_internal("avc", NULL, NULL, NULL, NULL);
 }
diff --git a/src/avc_sidtab.c b/src/avc_sidtab.c
index e396a938..3303537b 100644
--- a/src/avc_sidtab.c
+++ b/src/avc_sidtab.c
@@ -13,6 +13,7 @@
 #include "avc_sidtab.h"
 #include "avc_internal.h"
 
+ignore_unsigned_overflow_
 static inline unsigned sidtab_hash(const char * key)
 {
 	const char *p;
diff --git a/src/label.c b/src/label.c
index 4a7c6e6d..d2e703ef 100644
--- a/src/label.c
+++ b/src/label.c
@@ -60,7 +60,8 @@ static inline struct selabel_digest *selabel_is_digest_set
 {
 	struct selabel_digest *digest = NULL;
 
-	while (n--) {
+	while (n) {
+		n--;
 		if (opts[n].type == SELABEL_OPT_DIGEST &&
 					    opts[n].value == (char *)1) {
 			digest = calloc(1, sizeof(*digest));
@@ -112,9 +113,11 @@ static void selabel_digest_fini(struct selabel_digest *ptr)
 static inline int selabel_is_validate_set(const struct selinux_opt *opts,
 					  unsigned n)
 {
-	while (n--)
+	while (n) {
+		n--;
 		if (opts[n].type == SELABEL_OPT_VALIDATE)
 			return !!opts[n].value;
+	}
 
 	return 0;
 }
diff --git a/src/label_backends_android.c b/src/label_backends_android.c
index 7ddacdbe..33a17236 100644
--- a/src/label_backends_android.c
+++ b/src/label_backends_android.c
@@ -152,7 +152,8 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 	struct stat sb;
 
 	/* Process arguments */
-	while (n--)
+	while (n) {
+		n--;
 		switch (opts[n].type) {
 		case SELABEL_OPT_PATH:
 			path = opts[n].value;
@@ -165,6 +166,7 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 			path = opts[n].value;
 			break;
 		}
+	}
 
 	if (!path)
 		return -1;
diff --git a/src/label_db.c b/src/label_db.c
index 2daf1770..2ff10b2f 100644
--- a/src/label_db.c
+++ b/src/label_db.c
@@ -263,7 +263,8 @@ db_init(const struct selinux_opt *opts, unsigned nopts,
 	 *   the default one. If RDBMS is not SE-PostgreSQL, it may need to
 	 *   specify an explicit specfile for database objects.
 	 */
-	while (nopts--) {
+	while (nopts) {
+		nopts--;
 		switch (opts[nopts].type) {
 		case SELABEL_OPT_PATH:
 			path = opts[nopts].value;
diff --git a/src/label_file.c b/src/label_file.c
index 315298b3..3b2bda97 100644
--- a/src/label_file.c
+++ b/src/label_file.c
@@ -801,7 +801,8 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 	int status = -1, baseonly = 0;
 
 	/* Process arguments */
-	while (n--)
+	while (n) {
+		n--;
 		switch(opts[n].type) {
 		case SELABEL_OPT_PATH:
 			path = opts[n].value;
@@ -820,6 +821,7 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 			baseonly = !!opts[n].value;
 			break;
 		}
+	}
 
 #if !defined(BUILD_HOST) && !defined(ANDROID)
 	char subs_file[PATH_MAX + 1];
diff --git a/src/label_media.c b/src/label_media.c
index 4c987988..fad5ea6d 100644
--- a/src/label_media.c
+++ b/src/label_media.c
@@ -80,7 +80,8 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 	struct stat sb;
 
 	/* Process arguments */
-	while (n--)
+	while (n) {
+		n--;
 		switch(opts[n].type) {
 		case SELABEL_OPT_PATH:
 			path = opts[n].value;
@@ -93,6 +94,7 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 			path = opts[n].value;
 			break;
 		}
+}
 
 	/* Open the specification file. */
 	if (!path)
diff --git a/src/label_x.c b/src/label_x.c
index f332dcb6..bf569ca5 100644
--- a/src/label_x.c
+++ b/src/label_x.c
@@ -107,7 +107,8 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 	struct stat sb;
 
 	/* Process arguments */
-	while (n--)
+	while (n) {
+		n--;
 		switch(opts[n].type) {
 		case SELABEL_OPT_PATH:
 			path = opts[n].value;
@@ -120,6 +121,7 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
 			path = opts[n].value;
 			break;
 		}
+	}
 
 	/* Open the specification file. */
 	if (!path)
diff --git a/src/selinux_internal.h b/src/selinux_internal.h
index af69ff04..b134808e 100644
--- a/src/selinux_internal.h
+++ b/src/selinux_internal.h
@@ -102,4 +102,15 @@ size_t strlcpy(char *dest, const char *src, size_t size);
 size_t strlcpy(char *dest, const char *src, size_t size);
 #endif
 
+/* Use to ignore intentional unsigned under- and overflows while running under UBSAN. */
+#if defined(__clang__) && defined(__clang_major__) && (__clang_major__ >= 4)
+#if (__clang_major__ >= 12)
+#define ignore_unsigned_overflow_        __attribute__((no_sanitize("unsigned-integer-overflow", "unsigned-shift-base")))
+#else
+#define ignore_unsigned_overflow_        __attribute__((no_sanitize("unsigned-integer-overflow")))
+#endif
+#else
+#define ignore_unsigned_overflow_
+#endif
+
 #endif /* SELINUX_INTERNAL_H_ */
diff --git a/src/seusers.c b/src/seusers.c
index 16d69347..5a521f81 100644
--- a/src/seusers.c
+++ b/src/seusers.c
@@ -99,7 +99,7 @@ int require_seusers  = 0;
 
 static gid_t get_default_gid(const char *name) {
 	struct passwd pwstorage, *pwent = NULL;
-	gid_t gid = -1;
+	gid_t gid = (gid_t)-1;
 	/* Allocate space for the getpwnam_r buffer */
 	long rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
 	if (rbuflen <= 0) return -1;
diff --git a/src/sha1.c b/src/sha1.c
index 9d51e04a..452b0cc2 100644
--- a/src/sha1.c
+++ b/src/sha1.c
@@ -26,6 +26,8 @@
 #include "sha1.h"
 #include <memory.h>
 
+#include "selinux_internal.h"
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 //  TYPES
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -62,6 +64,7 @@ typedef union
 //
 //  Hash a single 512-bit block. This is the core of the algorithm
 ///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+ignore_unsigned_overflow_
 static
 void
     TransformFunction
diff --git a/src/stringrep.c b/src/stringrep.c
index d2237d1c..1b460224 100644
--- a/src/stringrep.c
+++ b/src/stringrep.c
@@ -337,13 +337,15 @@ void print_access_vector(security_class_t tclass, access_vector_t av)
 
 	printf(" {");
 
-	while (av) {
+	for (;;) {
 		if (av & bit) {
 			permstr = security_av_perm_to_string(tclass, bit);
 			if (!permstr)
 				break;
 			printf(" %s", permstr);
 			av &= ~bit;
+			if (!av)
+				break;
 		}
 		bit <<= 1;
 	}
-- 
2.33.0