diff --git a/parser/immunix.h b/parser/immunix.h index 357a2d16a..319723c52 100644 --- a/parser/immunix.h +++ b/parser/immunix.h @@ -175,6 +175,28 @@ static inline int is_merged_x_consistent(int a, int b) return 1; } +/* Arbitrary max and minimum priority that userspace can specify, + * internally we handle up to MAX_INTERNAL_PRIORITY and + * MIN_INTERNAL_PRIORITY. Do not ever allow INT_MAX, or INT_MIN + * because cmp uses subtraction and it can cause overflow. Ensure we + * don't over/underflow make internal max/min one more than allowed on + * rules. + * + * see + * note on mediates_priority + */ +#define MIN_POLICY_PRIORITY (-1000) +#define MAX_POLICY_PRIORITY (1000) + +/* internally we need a priority that any policy based rule can override + * and a priority that no policy based rule can override. These are + * used on rules encoding what abi/classes are supported by the + * compiled policy. + */ +#define MIN_INTERNAL_PRIORITY (MIN_POLICY_PRIORITY - 1) +#define MAX_INTERNAL_PRIORITY (MAX_POLICY_PRIORITY + 1) + + #endif /* ! _IMMUNIX_H */ /* LocalWords: MMAP diff --git a/parser/libapparmor_re/hfa.h b/parser/libapparmor_re/hfa.h index 3c6afb071..1ba0f2cb2 100644 --- a/parser/libapparmor_re/hfa.h +++ b/parser/libapparmor_re/hfa.h @@ -52,7 +52,7 @@ ostream &operator<<(ostream &os, State &state); class perms_t { public: - perms_t(void): priority(INT_MIN), allow(0), deny(0), prompt(0), audit(0), quiet(0), exact(0) { }; + perms_t(void): priority(MIN_INTERNAL_PRIORITY), allow(0), deny(0), prompt(0), audit(0), quiet(0), exact(0) { }; bool is_accept(void) { return (allow | deny | prompt | audit | quiet); } @@ -68,7 +68,7 @@ public: } void clear(void) { - priority = INT_MIN; + priority = MIN_INTERNAL_PRIORITY; allow = deny = prompt = audit = quiet = exact = 0; } void clear(int p) { diff --git a/parser/parser.h b/parser/parser.h index 6f0425c81..ca7274f25 100644 --- a/parser/parser.h +++ b/parser/parser.h @@ -53,12 +53,6 @@ using namespace std; */ extern int parser_token; -/* Arbitrary max and minimum priority that userspace can specify, internally - * we handle up to INT_MAX and INT_MIN. Do not ever allow INT_MAX, see - * note on mediates_priority - */ -#define MAX_PRIORITY 1000 -#define MIN_PRIORITY -1000 #define WARN_RULE_NOT_ENFORCED 0x1 #define WARN_RULE_DOWNGRADED 0x2 diff --git a/parser/parser_regex.c b/parser/parser_regex.c index 7810458d5..e4d6e012d 100644 --- a/parser/parser_regex.c +++ b/parser/parser_regex.c @@ -1093,9 +1093,10 @@ static const char *deny_file = ".*"; * * Note: it turns out the above bug does exist for dbus rules in parsers * that do not support priority, and we don't have a way to fix it. - * We fix it here by capping user specified priority to be < INT_MAX. + * We fix it here by capping user specified priority to be less than + * MAX_INTERNAL_PRIORITY. */ -static int mediates_priority = INT_MAX; +static int mediates_priority = MAX_INTERNAL_PRIORITY; /* some rule types unfortunately encoded permissions on the class byte * to fix the above bug, they need a different solution. The generic @@ -1106,7 +1107,7 @@ static int mediates_priority = INT_MAX; * and it is guaranteed to have the same priority as the highest priority * rule. */ -static int perms_onclass_mediates_priority = INT_MIN; +static int perms_onclass_mediates_priority = MIN_INTERNAL_PRIORITY; int process_profile_policydb(Profile *prof) { diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y index 7bc5067ef..d1fe72d2e 100644 --- a/parser/parser_yacc.y +++ b/parser/parser_yacc.y @@ -640,10 +640,10 @@ opt_priority: { $$ = 0; } yyerror("invalid priority %s", $3); free($3); /* see note on mediates_priority */ - if (tmp > MAX_PRIORITY) - yyerror("invalid priority %l > %d", tmp, MAX_PRIORITY); - if (tmp < MIN_PRIORITY) - yyerror("invalid priority %l > %d", tmp, MIN_PRIORITY); + if (tmp > MAX_POLICY_PRIORITY) + yyerror("invalid priority %l > %d", tmp, MAX_POLICY_PRIORITY); + if (tmp < MIN_POLICY_PRIORITY) + yyerror("invalid priority %l > %d", tmp, MIN_POLICY_PRIORITY); $$ = tmp; } diff --git a/parser/tst/simple_tests/file/priority/bad_1.sd b/parser/tst/simple_tests/file/priority/bad_1.sd new file mode 100644 index 000000000..26cf4d553 --- /dev/null +++ b/parser/tst/simple_tests/file/priority/bad_1.sd @@ -0,0 +1,7 @@ +# +#=Description to rule priority out of low end of range +#=EXRESULT FAIL +# +/usr/bin/foo { + priority=-1001 /usr/bin/foo r, +} diff --git a/parser/tst/simple_tests/file/priority/bad_2.sd b/parser/tst/simple_tests/file/priority/bad_2.sd new file mode 100644 index 000000000..84ab2df60 --- /dev/null +++ b/parser/tst/simple_tests/file/priority/bad_2.sd @@ -0,0 +1,7 @@ +# +#=Description basic file rule priority outside high end of range. +#=EXRESULT FAIL +# +/usr/bin/foo { + priority=1001 /usr/bin/foo r, +}