Received-SPF: pass (google.com: domain of linux-kernel+bounces-66193-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 14 Feb 2024 16:45:42 -0800
From: Kees Cook <keescook@chromium.org>
To: Casey Schaufler <casey@schaufler-ca.com>
Cc: Jann Horn <jannh@google.com>, Paul Moore <paul@paul-moore.com>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	John Johansen <john.johansen@canonical.com>,
	linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] security: fix integer overflow in lsm_set_self_attr()
 syscall
Message-ID: <202402141644.808307E71@keescook>
References: <20240214160538.1086089-1-jannh@google.com>
 <a4a77e0e-8d5e-4c9d-aff4-9fe0d8b89cf0@schaufler-ca.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a4a77e0e-8d5e-4c9d-aff4-9fe0d8b89cf0@schaufler-ca.com>

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...

-Kees

-- 
Kees Cook