All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian Göttsche" <cgzones@googlemail.com>
To: selinux@vger.kernel.org
Subject: [PATCH 10/11] libselinux: enable usage with pedantic UB sanitizers
Date: Tue, 19 Dec 2023 17:09:32 +0100	[thread overview]
Message-ID: <20231219160943.334370-10-cgzones@googlemail.com> (raw)
In-Reply-To: <20231219160943.334370-1-cgzones@googlemail.com>

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

diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
index 5e1c036e..ce87ac16 100644
--- a/libselinux/src/avc.c
+++ b/libselinux/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/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c
index e396a938..3303537b 100644
--- a/libselinux/src/avc_sidtab.c
+++ b/libselinux/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)
 {
 	unsigned int hash = 5381;
diff --git a/libselinux/src/label.c b/libselinux/src/label.c
index 4a7c6e6d..d2e703ef 100644
--- a/libselinux/src/label.c
+++ b/libselinux/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) {
 			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/libselinux/src/label_backends_android.c b/libselinux/src/label_backends_android.c
index 7ddacdbe..33a17236 100644
--- a/libselinux/src/label_backends_android.c
+++ b/libselinux/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,
 			errno = EINVAL;
 			return -1;
 		}
+	}
 
 	if (!path)
 		return -1;
diff --git a/libselinux/src/label_db.c b/libselinux/src/label_db.c
index 2daf1770..2ff10b2f 100644
--- a/libselinux/src/label_db.c
+++ b/libselinux/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/libselinux/src/label_file.c b/libselinux/src/label_file.c
index 315298b3..3b2bda97 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -68,7 +68,7 @@ static int find_stem_from_file(struct saved_data *data, const char *key)
 /*
  * hash calculation and key comparison of hash table
  */
-
+ignore_unsigned_overflow_
 static unsigned int symhash(hashtab_t h, const_hashtab_key_t key)
 {
 	const struct chkdups_key *k = (const struct chkdups_key *)key;
@@ -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,
 			errno = EINVAL;
 			return -1;
 		}
+	}
 
 #if !defined(BUILD_HOST) && !defined(ANDROID)
 	char subs_file[PATH_MAX + 1];
diff --git a/libselinux/src/label_media.c b/libselinux/src/label_media.c
index 4c987988..fad5ea6d 100644
--- a/libselinux/src/label_media.c
+++ b/libselinux/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,
 			errno = EINVAL;
 			return -1;
 		}
+}
 
 	/* Open the specification file. */
 	if (!path)
diff --git a/libselinux/src/label_x.c b/libselinux/src/label_x.c
index f332dcb6..bf569ca5 100644
--- a/libselinux/src/label_x.c
+++ b/libselinux/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,
 			errno = EINVAL;
 			return -1;
 		}
+	}
 
 	/* Open the specification file. */
 	if (!path)
diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
index af69ff04..b134808e 100644
--- a/libselinux/src/selinux_internal.h
+++ b/libselinux/src/selinux_internal.h
@@ -102,4 +102,15 @@ size_t strlcpy(char *dest, const char *src, size_t size);
 void *reallocarray(void *ptr, size_t nmemb, 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/libselinux/src/seusers.c b/libselinux/src/seusers.c
index 16d69347..5a521f81 100644
--- a/libselinux/src/seusers.c
+++ b/libselinux/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 */
 	char *rbuf = NULL;
 	long rbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
diff --git a/libselinux/src/sha1.c b/libselinux/src/sha1.c
index 9d51e04a..452b0cc2 100644
--- a/libselinux/src/sha1.c
+++ b/libselinux/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/libselinux/src/stringrep.c b/libselinux/src/stringrep.c
index d2237d1c..1b460224 100644
--- a/libselinux/src/stringrep.c
+++ b/libselinux/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.43.0


  parent reply	other threads:[~2023-12-19 16:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-19 16:09 [PATCH 01/11] libselinux/man: mention errno for regex compilation failure Christian Göttsche
2023-12-19 16:09 ` [PATCH 02/11] libselinux/man: sync selinux_check_securetty_context(3) Christian Göttsche
2023-12-19 16:09 ` [PATCH 03/11] libselinux/utils: free allocated resources Christian Göttsche
2023-12-19 16:09 ` [PATCH 04/11] libselinux/utils: improve compute_av output Christian Göttsche
2023-12-19 16:09 ` [PATCH 05/11] libselinux: align SELABEL_OPT_DIGEST usage with man page Christian Göttsche
2023-12-19 16:09 ` [PATCH 06/11] libselinux: fail selabel_open(3) on invalid option Christian Göttsche
2023-12-19 16:09 ` [PATCH 07/11] libselinux: use logging wrapper in getseuser(3) and get_default_context(3) family Christian Göttsche
2023-12-19 16:09 ` [PATCH 08/11] libselinux: support huge passwd/group entries Christian Göttsche
2023-12-19 16:09 ` [PATCH 09/11] libsemanage: support huge passwd entries Christian Göttsche
2023-12-19 16:09 ` Christian Göttsche [this message]
2023-12-19 16:09 ` [PATCH 11/11] setfiles: avoid unsigned integer underflow Christian Göttsche
2024-01-05 19:15 ` [PATCH 01/11] libselinux/man: mention errno for regex compilation failure James Carter
2024-01-25 19:55   ` James Carter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231219160943.334370-10-cgzones@googlemail.com \
    --to=cgzones@googlemail.com \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.