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@xxxxxxxxxxxxxx>
---
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
