On 9/16/19 4:30 PM, Mike Palmiotto wrote:
Commit c19395d72295f5e69275d98df5db22dfdf214b6c fixed some handling of unknown
classes/permissions, but missed the case where an unknown permission is loaded
and then subsequently logged, either via denial or auditallow. If a permission
set has some valid values mixed with unknown values, say `{ read write foo }`,
a check on `{ read write foo }` would fail to log the entire set.
To fix this, skip over the bad permissions/classes when expanding them to
strings. The unknowns should be logged during `selinux_set_mapping`, so
there is no need for further logging of the actual unknown permissions.
It seems like we have a similar pattern in print_access_vector(),
avc_dump_av(); if security_av_perm_to_string() returns NULL, we break
out of the loops and skip the remaining bits. We do however log the hex
value of the any remaining bits set in the av, so we don't lose all
information. I think we can still take this patch as is but was
wondering whether the other cases ought to be similarly handled?
Signed-off-by: Mike Palmiotto <mike.palmiotto@xxxxxxxxxxxxxxx>
---
libselinux/src/stringrep.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/libselinux/src/stringrep.c b/libselinux/src/stringrep.c
index ad29f76d..b9bf3ee6 100644
--- a/libselinux/src/stringrep.c
+++ b/libselinux/src/stringrep.c
@@ -268,7 +268,7 @@ const char *security_av_perm_to_string(security_class_t tclass,
int security_av_string(security_class_t tclass, access_vector_t av, char **res)
{
- unsigned int i = 0;
+ unsigned int i;
size_t len = 5;
access_vector_t tmp = av;
int rc = 0;
@@ -276,19 +276,12 @@ int security_av_string(security_class_t tclass, access_vector_t av, char **res)
char *ptr;
/* first pass computes the required length */
- while (tmp) {
+ for (i = 0; tmp; tmp >>= 1, i++) {
if (tmp & 1) {
str = security_av_perm_to_string(tclass, av & (1<<i));
if (str)
len += strlen(str) + 1;
- else {
- rc = -1;
- errno = EINVAL;
- goto out;
- }
}
- tmp >>= 1;
- i++;
}
*res = malloc(len);
@@ -298,7 +291,6 @@ int security_av_string(security_class_t tclass, access_vector_t av, char **res)
}
/* second pass constructs the string */
- i = 0;
tmp = av;
ptr = *res;
@@ -308,12 +300,12 @@ int security_av_string(security_class_t tclass, access_vector_t av, char **res)
}
ptr += sprintf(ptr, "{ ");
- while (tmp) {
- if (tmp & 1)
- ptr += sprintf(ptr, "%s ", security_av_perm_to_string(
- tclass, av & (1<<i)));
- tmp >>= 1;
- i++;
+ for (i = 0; tmp; tmp >>= 1, i++) {
+ if (tmp & 1) {
+ str = security_av_perm_to_string(tclass, av & (1<<i));
+ if (str)
+ ptr += sprintf(ptr, "%s ", str);
+ }
}
sprintf(ptr, "}");
out:
