From 4efff35bf8991fcdda3f16e65a036826b9b5cf5f Mon Sep 17 00:00:00 2001 From: Christian Boltz Date: Tue, 13 Nov 2018 17:53:49 +0100 Subject: [PATCH] parse_profile_data(): Ensure last line in a profile is valid 'lastline' gets merged into 'line' (and reset to None) when reading the next line. If 'lastline' isn't empty after reading the whole profile, this means there's something unparseable at the end of the profile, therefore parse_profile_data() should error out. Also remove some simple_tests testcases from the 'exception_not_raised' list - they only didn't raise the exception because the invalid rule was the last line in the affected profile. Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-) discovering this bug while adding some xattr testcases that surprisingly didn't fail in the tools. --- utils/apparmor/aa.py | 5 +++++ utils/test/test-parser-simple-tests.py | 10 ---------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/utils/apparmor/aa.py b/utils/apparmor/aa.py index 30ad07af3..fd44c8092 100644 --- a/utils/apparmor/aa.py +++ b/utils/apparmor/aa.py @@ -2544,6 +2544,11 @@ def parse_profile_data(data, file, do_include): else: raise AppArmorException(_('Syntax Error: Unknown line found in file %(file)s line %(lineno)s:\n %(line)s') % { 'file': file, 'lineno': lineno + 1, 'line': line }) + if lastline: + # lastline gets merged into line (and reset to None) when reading the next line. + # If it isn't empty, this means there's something unparseable at the end of the profile + raise AppArmorException(_('Syntax Error: Unknown line found in file %(file)s line %(lineno)s:\n %(line)s') % { 'file': file, 'lineno': lineno + 1, 'line': lastline }) + # Below is not required I'd say if not do_include: for hatglob in cfg['required_hats'].keys(): diff --git a/utils/test/test-parser-simple-tests.py b/utils/test/test-parser-simple-tests.py index cee6e2529..19453d641 100644 --- a/utils/test/test-parser-simple-tests.py +++ b/utils/test/test-parser-simple-tests.py @@ -40,11 +40,6 @@ skip_startswith = ( # testcases that should raise an exception, but don't exception_not_raised = [ # most abi/bad_* aren't detected as bad by the basic implementation in the tools - 'abi/bad_1.sd', - 'abi/bad_2.sd', - 'abi/bad_3.sd', - 'abi/bad_4.sd', - 'abi/bad_5.sd', 'abi/bad_10.sd', 'abi/bad_11.sd', 'abi/bad_12.sd', @@ -155,13 +150,9 @@ exception_not_raised = [ 'unix/bad_regex_04.sd', 'unix/bad_shutdown_1.sd', 'unix/bad_shutdown_2.sd', - 'vars/boolean/boolean_bad_1.sd', 'vars/boolean/boolean_bad_2.sd', 'vars/boolean/boolean_bad_3.sd', 'vars/boolean/boolean_bad_4.sd', - 'vars/boolean/boolean_bad_6.sd', - 'vars/boolean/boolean_bad_7.sd', - 'vars/boolean/boolean_bad_8.sd', 'vars/vars_bad_3.sd', 'vars/vars_bad_4.sd', 'vars/vars_bad_5.sd', @@ -200,7 +191,6 @@ exception_not_raised = [ 'vars/vars_recursion_2.sd', 'vars/vars_recursion_3.sd', 'vars/vars_recursion_4.sd', - 'vars/vars_simple_assignment_10.sd', 'vars/vars_simple_assignment_3.sd', 'vars/vars_simple_assignment_8.sd', 'vars/vars_simple_assignment_9.sd',