On 5/28/19 8:32 AM, Jann Horn wrote: > A packed AppArmor policy contains null-terminated tag strings that are read > by unpack_nameX(). However, unpack_nameX() uses string functions on them > without ensuring that they are actually null-terminated, potentially > leading to out-of-bounds accesses. > > Make sure that the tag string is null-terminated before passing it to > strcmp(). > > Cc: [email protected] > Signed-off-by: Jann Horn <[email protected]>
gah! yes! Acked-by: John Johansen <[email protected]> > --- > Warning: The existence of this bug has not been verified at runtime, and > the patch is compile-tested only. I noticed this while browsing through > the code, but didn't want to spend the time necessary to figure out how > to actually test this at runtime. > > > security/apparmor/policy_unpack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/apparmor/policy_unpack.c > b/security/apparmor/policy_unpack.c > index f6c2bcb2ab14..33041c4fb69f 100644 > --- a/security/apparmor/policy_unpack.c > +++ b/security/apparmor/policy_unpack.c > @@ -276,7 +276,7 @@ static bool unpack_nameX(struct aa_ext *e, enum aa_code > code, const char *name) > char *tag = NULL; > size_t size = unpack_u16_chunk(e, &tag); > /* if a name is specified it must match. otherwise skip tag */ > - if (name && (!size || strcmp(name, tag))) > + if (name && (!size || tag[size-1] != '\0' || strcmp(name, tag))) > goto fail; > } else if (name) { > /* if a name is specified and there is no name tag fail */ >
