mir/apparmor
2
0
Fork 0
mirror of https://gitlab.com/apparmor/apparmor synced 2025-08-22 01:57:43 +00:00
Code Issues Releases Wiki Activity

Merge parser/variables: fix read-after-free in error case

Browse Source 
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

```

Signed-off-by: Steve Beattie <steve@nxnw.org>

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1747
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
This commit is contained in:
Georgia Garcia 2025-07-25 09:42:38 -03:00
commit e757ca8e14
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
parser/variable.cc
View File

@ -211,8 +211,6 @@ int variable::expand_by_alternation(char **name)
return 0; /* no var found, name is unchanged */
}
free(*name);
if (!prefix.empty() && prefix[prefix.size() - 1] == '/') {
/* make sure to not collapse / in the beginning of the path */
std::size_t found = prefix.find_first_not_of('/');
@ -228,6 +226,8 @@ int variable::expand_by_alternation(char **name)
return 1;
}
free(*name);
size_t setsize = ref->expanded.size();
auto i = ref->expanded.begin();