From f8b43d5ba9f476732ba256424e0f7df1e77be6aa Mon Sep 17 00:00:00 2001 From: Steve Beattie Date: Mon, 28 Mar 2011 10:52:02 -0700 Subject: [PATCH] The parser's lexer supports variables defined matching the regex '[[:alpha:]][[:alnum:]_]*' (i.e. a single alpha followed by any number of alphanumerics or underscores). Unfortunately, the code that expends variables inside a profile does not match this, it incorrectly matched '([[:alpha:]]|_)+' (one or more alphas or underscores). This patch corrects the behavior there as well as synchronizing the expected variable names in the apparmor.d manpage and apparmor.vim syntax file. It also adds unit tests and testcases to verify the behavior. Signed-off-by: Steve Beattie --- parser/apparmor.d.pod | 2 +- parser/parser_variable.c | 40 ++++++++++++++++++- .../simple_tests/vars/vars_alternation_1.sd | 9 +++++ .../simple_tests/vars/vars_alternation_2.sd | 9 +++++ parser/tst/simple_tests/vars/vars_bad_4.sd | 7 ++++ parser/tst/simple_tests/vars/vars_bad_5.sd | 7 ++++ .../vars/vars_file_evaluation_15.sd | 9 +++++ .../vars/vars_file_evaluation_16.sd | 7 ++++ utils/apparmor.vim | 2 +- 9 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 parser/tst/simple_tests/vars/vars_alternation_1.sd create mode 100644 parser/tst/simple_tests/vars/vars_alternation_2.sd create mode 100644 parser/tst/simple_tests/vars/vars_bad_4.sd create mode 100644 parser/tst/simple_tests/vars/vars_bad_5.sd create mode 100644 parser/tst/simple_tests/vars/vars_file_evaluation_15.sd create mode 100644 parser/tst/simple_tests/vars/vars_file_evaluation_16.sd diff --git a/parser/apparmor.d.pod b/parser/apparmor.d.pod index 73355c889..cbfa67643 100644 --- a/parser/apparmor.d.pod +++ b/parser/apparmor.d.pod @@ -83,7 +83,7 @@ B = (must start with '/' (after variable expansion), B have s B = ( 'r' | 'w' | 'l' | 'ix' | 'ux' | 'Ux' | 'px' | 'Px' | 'cx -> ' I | 'Cx -> ' I | 'm' ) [ I ... ] (not all combinations are allowed; see below.) -B = '@{' I [ I ... ] '}' +B = '@{' I [ ( I | '_' ) ... ] '}' B = I ('=' | '+=') (space separated values) diff --git a/parser/parser_variable.c b/parser/parser_variable.c index b4f703f34..e9427f56d 100644 --- a/parser/parser_variable.c +++ b/parser/parser_variable.c @@ -36,8 +36,14 @@ static inline char *get_var_end(char *var) while (*eptr) { if (*eptr == '}') return eptr; - if (!(*eptr == '_' || isalpha(*eptr))) - return NULL; /* invalid char */ + /* first character must be alpha */ + if (eptr == var) { + if (!isalpha(*eptr)) + return NULL; /* invalid char */ + } else { + if (!(*eptr == '_' || isalnum(*eptr))) + return NULL; /* invalid char */ + } eptr++; } return NULL; /* no terminating '}' */ @@ -317,6 +323,8 @@ int test_split_out_var(void) struct var_string *ret_struct; char *prefix = "abcdefg"; char *var = "boogie"; + char *var2 = "V4rW1thNum5"; + char *var3 = "boogie_board"; char *suffix = "suffixication"; /* simple case */ @@ -394,6 +402,34 @@ int test_split_out_var(void) MY_TEST(strcmp(ret_struct->suffix, suffix) == 0, "split out var 7 suffix"); free_var_string(ret_struct); + /* numeric char in var, should succeed */ + asprintf(&tst_string, "%s@{%s}%s", prefix, var2, suffix); + ret_struct = split_out_var(tst_string); + MY_TEST(ret_struct && strcmp(ret_struct->prefix, prefix) == 0, "split out numeric var prefix"); + MY_TEST(ret_struct && strcmp(ret_struct->var, var2) == 0, "split numeric var var"); + MY_TEST(ret_struct && strcmp(ret_struct->suffix, suffix) == 0, "split out numeric var suffix"); + free_var_string(ret_struct); + + /* numeric first char in var, should fail */ + asprintf(&tst_string, "%s@{6%s}%s", prefix, var2, suffix); + ret_struct = split_out_var(tst_string); + MY_TEST(ret_struct == NULL, "split out var - numeric first char in var"); + free_var_string(ret_struct); + + /* underscore char in var, should succeed */ + asprintf(&tst_string, "%s@{%s}%s", prefix, var3, suffix); + ret_struct = split_out_var(tst_string); + MY_TEST(ret_struct && strcmp(ret_struct->prefix, prefix) == 0, "split out underscore var prefix"); + MY_TEST(ret_struct && strcmp(ret_struct->var, var3) == 0, "split out underscore var"); + MY_TEST(ret_struct && strcmp(ret_struct->suffix, suffix) == 0, "split out underscore var suffix"); + free_var_string(ret_struct); + + /* underscore first char in var, should fail */ + asprintf(&tst_string, "%s@{_%s%s}%s", prefix, var2, var3, suffix); + ret_struct = split_out_var(tst_string); + MY_TEST(ret_struct == NULL, "split out var - underscore first char in var"); + free_var_string(ret_struct); + return rc; } int main(void) diff --git a/parser/tst/simple_tests/vars/vars_alternation_1.sd b/parser/tst/simple_tests/vars/vars_alternation_1.sd new file mode 100644 index 000000000..48bfcf556 --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_alternation_1.sd @@ -0,0 +1,9 @@ +#=DESCRIPTION reference variables in rules that also have alternations +#=EXRESULT + +@{FOO}=bar baz +@{BAR}=${FOO} blort + +/does/not/exist { + /does/not/{exist,notexist}/blah/@{BAR} r, +} diff --git a/parser/tst/simple_tests/vars/vars_alternation_2.sd b/parser/tst/simple_tests/vars/vars_alternation_2.sd new file mode 100644 index 000000000..f9058c9a4 --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_alternation_2.sd @@ -0,0 +1,9 @@ +#=DESCRIPTION reference variables in rules that also have alternations +#=EXRESULT + +@{FOO}=bar baz +@{BAR}=${FOO} blort + +/does/not/exist { + /does/not/@{BAR}/blah/{exist,notexist} r, +} diff --git a/parser/tst/simple_tests/vars/vars_bad_4.sd b/parser/tst/simple_tests/vars/vars_bad_4.sd new file mode 100644 index 000000000..4435dd236 --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_bad_4.sd @@ -0,0 +1,7 @@ +#=DESCRIPTION don't accept variables with leading underscores +#=EXRESULT FAIL +@{_FOO} = /foo /bar /baz /biff + +/usr/bin/foo { + /@{_FOO}/.foo/* r, +} diff --git a/parser/tst/simple_tests/vars/vars_bad_5.sd b/parser/tst/simple_tests/vars/vars_bad_5.sd new file mode 100644 index 000000000..34b87a7ed --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_bad_5.sd @@ -0,0 +1,7 @@ +#=DESCRIPTION don't accept variables with leading numeric +#=EXRESULT FAIL +@{4FOO} = /foo /bar /baz /biff + +/usr/bin/foo { + /@{4FOO}/.foo/* r, +} diff --git a/parser/tst/simple_tests/vars/vars_file_evaluation_15.sd b/parser/tst/simple_tests/vars/vars_file_evaluation_15.sd new file mode 100644 index 000000000..7a88062dc --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_file_evaluation_15.sd @@ -0,0 +1,9 @@ +#=DESCRIPTION simple expansions within file rules with numeric variable +#=EXRESULT PASS +@{FOO1} = /foo /bar /baz /biff +@{B1A1R1} = @{FOO1} + +/usr/bin/foo { + /@{FOO1}/.foo/* r, + /foo/@{B1A1R1}/.foo/* r, +} diff --git a/parser/tst/simple_tests/vars/vars_file_evaluation_16.sd b/parser/tst/simple_tests/vars/vars_file_evaluation_16.sd new file mode 100644 index 000000000..6de0f55d8 --- /dev/null +++ b/parser/tst/simple_tests/vars/vars_file_evaluation_16.sd @@ -0,0 +1,7 @@ +#=DESCRIPTION simple expansions within file rules with underscore variable +#=EXRESULT PASS +@{F_OO} = /foo /bar /baz /biff + +/usr/bin/foo { + /@{F_OO}/.foo/* r, +} diff --git a/utils/apparmor.vim b/utils/apparmor.vim index bb5e7e778..24053c8f2 100644 --- a/utils/apparmor.vim +++ b/utils/apparmor.vim @@ -113,7 +113,7 @@ syn match sdError /^.*$/ contains=sdComment "highlight all non-valid lines as er " This allows incorrect lines also and should be checked better. " This also (accidently ;-) includes variable definitions (@{FOO}=/bar) " TODO: make a separate pattern for variable definitions, then mark sdGlob as contained -syn match sdGlob /\v\?|\*|\{.*,.*\}|[[^\]]\+\]|\@\{[a-zA-Z_]*\}/ +syn match sdGlob /\v\?|\*|\{.*,.*\}|[[^\]]\+\]|\@\{[a-zA-Z][a-zA-Z0-9_]*\}/ syn match sdAlias /\v^alias\s+(\/|\@\{\S*\})\S*\s+-\>\s+(\/|\@\{\S*\})\S*\s*,(\s*$|(\s*#.*$)\@=)/ contains=sdGlob