cap: fix format-string vulnerability
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 2 May 2019 00:16:06 +0000 (20:16 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 2 May 2019 01:10:52 +0000 (21:10 -0400)
It looks like the old code was trying to use snprintf() to sanitize the
user-provided strings.  That worked, but then we passed it to error(),
and the string passed to error() is *itself* a format string.  So in
attempting to make things more secure, we botched it.

Note that from and key are not user-pointers, and we know they are
null-terminated.  Our printf family of functions (and thus our error())
can handle null-terminated strings made of user-data, but not user
*pointers.*  By default, snprintf() does an unbounded strnlen() on the
string pointer, which could look beyond the region checked by
is_user_raddr().

Reported-by: syzbot+871c0525c81bbe0e93a5@syzkaller.appspotmail.com
Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/drivers/dev/capability.c

index 6625086..f489e6f 100644 (file)
@@ -204,7 +204,6 @@ static size_t capwrite(struct chan *c, void *va, size_t n, off64_t m)
        uint8_t hash[Hashlen + 1] = {0};
        char *hashstr = NULL;
        char *key, *from, *to;
-       char err[256];
        int ret;
        ERRSTACK(1);
 
@@ -221,7 +220,8 @@ static size_t capwrite(struct chan *c, void *va, size_t n, off64_t m)
                break;
 
        case Quse:
-               /* copy key to avoid a fault in hmac_xx */
+               /* copy key to avoid a fault in hmac_xx and so we can enforce
+                * null termination. */
                cp = NULL;
                if (waserror()) {
                        kfree(cp);
@@ -250,11 +250,8 @@ static size_t capwrite(struct chan *c, void *va, size_t n, off64_t m)
                        error(EINVAL, "hash is wrong length");
 
                p = remcap((uint8_t *)hashstr);
-               if (p == NULL) {
-                       snprintf(err, sizeof(err), "invalid capability %s@%s",
-                                from, key);
-                       error(EINVAL, err);
-               }
+               if (p == NULL)
+                       error(EINVAL, "invalid capability %s@%s", from, key);
 
                kfree(hashstr);
                hashstr = NULL;