[apparmor] [patch] make parser's definition of allowed var names consistent

Steve Beattie steve at nxnw.org
Mon Mar 28 09:08:15 UTC 2011


[This patch is nominated for trunk and 2.6.2.]

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 <sbeattie at ubuntu.com>

=== modified file 'parser/parser_variable.c'
---
 parser/apparmor.d.pod                                   |    2 
 parser/parser_variable.c                                |   40 +++++++++++++++-
 parser/tst/simple_tests/vars/vars_bad_4.sd              |    7 ++
 parser/tst/simple_tests/vars/vars_bad_5.sd              |    7 ++
 parser/tst/simple_tests/vars/vars_file_evaluation_15.sd |    9 +++
 parser/tst/simple_tests/vars/vars_file_evaluation_16.sd |    7 ++
 utils/apparmor.vim                                      |    2 
 7 files changed, 70 insertions(+), 4 deletions(-)

Index: b/parser/parser_variable.c
===================================================================
--- a/parser/parser_variable.c
+++ b/parser/parser_variable.c
@@ -36,8 +36,14 @@ static inline char *get_var_end(char *va
 	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)
Index: b/parser/tst/simple_tests/vars/vars_bad_4.sd
===================================================================
--- /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,
+}
Index: b/parser/tst/simple_tests/vars/vars_file_evaluation_16.sd
===================================================================
--- /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,
+}
Index: b/parser/tst/simple_tests/vars/vars_bad_5.sd
===================================================================
--- /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,
+}
Index: b/parser/tst/simple_tests/vars/vars_file_evaluation_15.sd
===================================================================
--- /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,
+}
Index: b/parser/apparmor.d.pod
===================================================================
--- a/parser/apparmor.d.pod
+++ b/parser/apparmor.d.pod
@@ -83,7 +83,7 @@ B<FILEGLOB> = (must start with '/' (afte
 
 B<ACCESS> = ( 'r' | 'w' | 'l' | 'ix' | 'ux' | 'Ux' | 'px' | 'Px' | 'cx -> ' I<PROGRAMCHILD> | 'Cx -> ' I<PROGRAMCHILD> | 'm' ) [ I<ACCESS> ... ]  (not all combinations are allowed; see below.)
 
-B<VARIABLE> = '@{' I<ALPHA> [ I<ALPHANUMERIC> ... ] '}'
+B<VARIABLE> = '@{' I<ALPHA> [ ( I<ALPHANUMERIC> | '_' ) ... ] '}'
 
 B<VARIABLE ASSIGNMENT> = I<VARIABLE> ('=' | '+=') (space separated values)
 
Index: b/utils/apparmor.vim
===================================================================
--- a/utils/apparmor.vim
+++ b/utils/apparmor.vim
@@ -113,7 +113,7 @@ syn match sdError /^.*$/ contains=sdComm
 " 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
 

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20110328/e028caa9/attachment.pgp>


More information about the AppArmor mailing list