iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lsm: fix integer overflow in lsm_set_self_attr() syscall

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

```

struct lsm_ctx {
  uint64_t id;
  uint64_t flags;
  uint64_t len;
  uint64_t ctx_len;
  char ctx[];
};

int main(void) {
  size_t size = sizeof(struct lsm_ctx);
  struct lsm_ctx *ctx = malloc(size);
  ctx->id = 104/*LSM_ID_APPARMOR*/;
  ctx->flags = 0;
  ctx->len = size;
  ctx->ctx_len = -sizeof(struct lsm_ctx);
  syscall(
    460/*__NR_lsm_set_self_attr*/,
    /*attr=*/  100/*LSM_ATTR_CURRENT*/,
    /*ctx=*/   ctx,
    /*size=*/  size,
    /*flags=*/ 0
  );
}
```

Fixes: a04a119808 ("LSM: syscalls for current process attributes")
Signed-off-by: Jann Horn <jannh@google.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
[PM: subj tweak, removed ref to ASAN splat that isn't included]
Signed-off-by: Paul Moore <paul@paul-moore.com>
This commit is contained in:
Jann Horn 2024-02-14 17:05:38 +01:00 committed by Paul Moore
parent 5a287d3d2b
commit d8bdd795d3
1 changed files with 5 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
security/security.c
View File

@ -29,6 +29,7 @@
#include <linux/backing-dev.h>
#include <linux/string.h>
#include <linux/msg.h>
#include <linux/overflow.h>
#include <net/flow.h>
/* How many LSMs were built into the kernel? */
@ -4015,6 +4016,7 @@ int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx,
struct security_hook_list *hp;
struct lsm_ctx *lctx;
int rc = LSM_RET_DEFAULT(setselfattr);
u64 required_len;
if (flags)
return -EINVAL;
@ -4027,8 +4029,9 @@ int security_setselfattr(unsigned int attr, struct lsm_ctx __user *uctx,
if (IS_ERR(lctx))
return PTR_ERR(lctx);
if (size < lctx->len || size < lctx->ctx_len + sizeof(*lctx) ||
lctx->len < lctx->ctx_len + sizeof(*lctx)) {
if (size < lctx->len ||
check_add_overflow(sizeof(*lctx), lctx->ctx_len, &required_len) ||
lctx->len < required_len) {
rc = -EINVAL;
goto free_out;
}