[PATCH 1/2] libsepol: drop confusing BUG_ON macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <nicolas.iooss@xxxxxxx>
---
 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.28.0




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux