lsm: fix integer overflow in lsm_set_self_attr() syscall

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: a04a1198088a ("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>

diff --git a/security/security.c b/security/security.c
index 3aaad75c9ce8531b6f214ad8bd469d1c571908c7..7035ee35a393020303304741092e6b016e02669c 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -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;
        }