mirror of
https://gitlab.com/apparmor/apparmor
synced 2025-08-22 10:07:12 +00:00
parser/variables: fix read-after-free in error case
When variable expansion occurs, the expansion attempts to replace the memory location of the string containing the variable, and frees the string it is replacing. However, this occurs before the variable lookup occurs to determine if there is an appropriate declaration for the variable. When the failing expansion occurs in a profile name, this causes a read-after-free (followed by a double free) because the error handling path attempts to report the profile name in the error message. This can be reproduced like so, using the tst/simple_tests/vars/vars_profile_name_23.sd testcase: ``` $ ../apparmor_parser --config-file=./parser.conf -M features_files/features.all -S -I /home/sbeattie/git/apparmor/parser/tst/./simple_tests/ ./simple_tests/vars/vars_profile_name_23.sd Failed to find declaration for: @{FOO} ERROR expanding variables for profile #xQV, failed to load free(): double free detected in tcache 2 ``` Fix this by waiting to free the profile name field until after the variable declaration has successfully been looked up. This results in the test case reporting the following error: ``` $ ../apparmor_parser --config-file=./parser.conf -M features_files/features.all -S -I /home/sbeattie/git/apparmor/parser/tst/./simple_tests/ ./simple_tests/vars/vars_profile_name_23.sd Failed to find declaration for: @{FOO} ERROR expanding variables for profile /does/not/exist@{FOO}, failed to load ``` Fixes: dfbd2dc4b ("parser: refactor variables and symbols table into their own class") Signed-off-by: Steve Beattie <steve@nxnw.org> Ref: https://gitlab.com/apparmor/apparmor/-/merge_requests/1747
This commit is contained in:
parent
49cb0fe248
commit
6673be07aa
@ -211,8 +211,6 @@ int variable::expand_by_alternation(char **name)
|
|||||||
return 0; /* no var found, name is unchanged */
|
return 0; /* no var found, name is unchanged */
|
||||||
}
|
}
|
||||||
|
|
||||||
free(*name);
|
|
||||||
|
|
||||||
if (!prefix.empty() && prefix[prefix.size() - 1] == '/') {
|
if (!prefix.empty() && prefix[prefix.size() - 1] == '/') {
|
||||||
/* make sure to not collapse / in the beginning of the path */
|
/* make sure to not collapse / in the beginning of the path */
|
||||||
std::size_t found = prefix.find_first_not_of('/');
|
std::size_t found = prefix.find_first_not_of('/');
|
||||||
@ -228,6 +226,8 @@ int variable::expand_by_alternation(char **name)
|
|||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
free(*name);
|
||||||
|
|
||||||
size_t setsize = ref->expanded.size();
|
size_t setsize = ref->expanded.size();
|
||||||
auto i = ref->expanded.begin();
|
auto i = ref->expanded.begin();
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user