Re: [PATCH] security: fix integer overflow in lsm_set_self_attr() syscall

[Paul Moore]

On February 14, 2024 7:45:43 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> On Wed, Feb 14, 2024 at 08:53:52AM -0800, Casey Schaufler wrote:
> > On 2/14/2024 8:05 AM, Jann Horn wrote:
> > > security_setselfattr() has an integer overflow bug that leads to
> > > out-of-bounds access when userspace provides bogus input:
> > > `lctx->ctx_len + sizeof(*lctx)` is checked against `lctx->len` (and,
> > > redundantly, also against `size`), but there are no checks on
> > > `lctx->ctx_len`.
> > > Therefore, userspace can provide an `lsm_ctx` with `->ctx_len` set to a
> > > value between `-sizeof(struct lsm_ctx)` and -1, and this bogus `->ctx_len`
> > > will then be passed to an LSM module as a buffer length, causing LSM
> > > modules to perform out-of-bounds accesses.
> > >
> > > The following reproducer will demonstrate this under ASAN (if AppArmor is
> > > loaded as an LSM):
> > > ```
> > > #define _GNU_SOURCE
> > > #include <unistd.h>
> > > #include <stdint.h>
> > > #include <stdlib.h>
> > > #include <sys/syscall.h>
> > >
> > > struct lsm_ctx {
> > > uint64_t id;
> > > uint64_t flags;
> > > uint64_t len;
> > > uint64_t ctx_len;
>
> Do we want to take the opportunity to reduce this to u32 for len and u32
> for ctx_len? All FS operations are limited to INT_MAX anyway...

Not at this point, no.

--
paul-moore.com