From 64387cb37379fc8f135eeecd2bd9fdf3c591c763 Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Sat, 3 Oct 2020 15:34:19 +0200 Subject: [PATCH] libsepol: drop confusing BUG_ON macro Contrary to Linux kernel, BUG_ON() does not halt the execution, in libsepol/src/services.c. Instead it displays an error message and continues the execution. This means that this code does not prevent an out-of-bound write from happening: case CEXPR_AND: BUG_ON(sp < 1); sp--; s[sp] &= s[sp + 1]; Use if(...){BUG();rc=-EINVAL;goto out;} constructions instead, to make sure that the array access is always in-bound. This issue has been found using clang's static analyzer: https://558-118970575-gh.circle-artifacts.com/0/output-scan-build/2020-10-02-065849-6375-1/report-50a861.html#EndPath Signed-off-by: Nicolas Iooss --- libsepol/src/services.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/libsepol/src/services.c b/libsepol/src/services.c index 90da1f4efef3..beb0711f6680 100644 --- a/libsepol/src/services.c +++ b/libsepol/src/services.c @@ -67,7 +67,6 @@ #include "flask.h" #define BUG() do { ERR(NULL, "Badness at %s:%d", __FILE__, __LINE__); } while (0) -#define BUG_ON(x) do { if (x) ERR(NULL, "Badness at %s:%d", __FILE__, __LINE__); } while (0) static int selinux_enforcing = 1; @@ -469,18 +468,30 @@ static int constraint_expr_eval_reason(context_struct_t *scontext, /* Now process each expression of the constraint */ switch (e->expr_type) { case CEXPR_NOT: - BUG_ON(sp < 0); + if (sp < 0) { + BUG(); + rc = -EINVAL; + goto out; + } s[sp] = !s[sp]; cat_expr_buf(expr_list[expr_counter], "not"); break; case CEXPR_AND: - BUG_ON(sp < 1); + if (sp < 1) { + BUG(); + rc = -EINVAL; + goto out; + } sp--; s[sp] &= s[sp + 1]; cat_expr_buf(expr_list[expr_counter], "and"); break; case CEXPR_OR: - BUG_ON(sp < 1); + if (sp < 1) { + BUG(); + rc = -EINVAL; + goto out; + } sp--; s[sp] |= s[sp + 1]; cat_expr_buf(expr_list[expr_counter], "or"); -- 2.29.0.rc2