rxrpc: Fix several cases where a padded len isn't checked in ticket decode
authorDavid Howells <dhowells@redhat.com>
Wed, 14 Jun 2017 23:12:24 +0000 (00:12 +0100)
committerDavid S. Miller <davem@davemloft.net>
Thu, 15 Jun 2017 18:23:44 +0000 (14:23 -0400)
This fixes CVE-2017-7482.

When a kerberos 5 ticket is being decoded so that it can be loaded into an
rxrpc-type key, there are several places in which the length of a
variable-length field is checked to make sure that it's not going to
overrun the available data - but the data is padded to the nearest
four-byte boundary and the code doesn't check for this extra.  This could
lead to the size-remaining variable wrapping and the data pointer going
over the end of the buffer.

Fix this by making the various variable-length data checks use the padded
length.

Reported-by: 石磊 <shilei-c@360.cn>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Marc Dionne <marc.c.dionne@auristor.com>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/rxrpc/key.c

index 0a4e28477ad94012b4e7aeb526711076e6259995..54369225766ef8e43ff94f21b4fd670b2cc233a3 100644 (file)
@@ -217,7 +217,7 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
                                       unsigned int *_toklen)
 {
        const __be32 *xdr = *_xdr;
-       unsigned int toklen = *_toklen, n_parts, loop, tmp;
+       unsigned int toklen = *_toklen, n_parts, loop, tmp, paddedlen;
 
        /* there must be at least one name, and at least #names+1 length
         * words */
@@ -247,16 +247,16 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
                toklen -= 4;
                if (tmp <= 0 || tmp > AFSTOKEN_STRING_MAX)
                        return -EINVAL;
-               if (tmp > toklen)
+               paddedlen = (tmp + 3) & ~3;
+               if (paddedlen > toklen)
                        return -EINVAL;
                princ->name_parts[loop] = kmalloc(tmp + 1, GFP_KERNEL);
                if (!princ->name_parts[loop])
                        return -ENOMEM;
                memcpy(princ->name_parts[loop], xdr, tmp);
                princ->name_parts[loop][tmp] = 0;
-               tmp = (tmp + 3) & ~3;
-               toklen -= tmp;
-               xdr += tmp >> 2;
+               toklen -= paddedlen;
+               xdr += paddedlen >> 2;
        }
 
        if (toklen < 4)
@@ -265,16 +265,16 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
        toklen -= 4;
        if (tmp <= 0 || tmp > AFSTOKEN_K5_REALM_MAX)
                return -EINVAL;
-       if (tmp > toklen)
+       paddedlen = (tmp + 3) & ~3;
+       if (paddedlen > toklen)
                return -EINVAL;
        princ->realm = kmalloc(tmp + 1, GFP_KERNEL);
        if (!princ->realm)
                return -ENOMEM;
        memcpy(princ->realm, xdr, tmp);
        princ->realm[tmp] = 0;
-       tmp = (tmp + 3) & ~3;
-       toklen -= tmp;
-       xdr += tmp >> 2;
+       toklen -= paddedlen;
+       xdr += paddedlen >> 2;
 
        _debug("%s/...@%s", princ->name_parts[0], princ->realm);
 
@@ -293,7 +293,7 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td,
                                         unsigned int *_toklen)
 {
        const __be32 *xdr = *_xdr;
-       unsigned int toklen = *_toklen, len;
+       unsigned int toklen = *_toklen, len, paddedlen;
 
        /* there must be at least one tag and one length word */
        if (toklen <= 8)
@@ -307,15 +307,17 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td,
        toklen -= 8;
        if (len > max_data_size)
                return -EINVAL;
+       paddedlen = (len + 3) & ~3;
+       if (paddedlen > toklen)
+               return -EINVAL;
        td->data_len = len;
 
        if (len > 0) {
                td->data = kmemdup(xdr, len, GFP_KERNEL);
                if (!td->data)
                        return -ENOMEM;
-               len = (len + 3) & ~3;
-               toklen -= len;
-               xdr += len >> 2;
+               toklen -= paddedlen;
+               xdr += paddedlen >> 2;
        }
 
        _debug("tag %x len %x", td->tag, td->data_len);
@@ -387,7 +389,7 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
                                    const __be32 **_xdr, unsigned int *_toklen)
 {
        const __be32 *xdr = *_xdr;
-       unsigned int toklen = *_toklen, len;
+       unsigned int toklen = *_toklen, len, paddedlen;
 
        /* there must be at least one length word */
        if (toklen <= 4)
@@ -399,6 +401,9 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
        toklen -= 4;
        if (len > AFSTOKEN_K5_TIX_MAX)
                return -EINVAL;
+       paddedlen = (len + 3) & ~3;
+       if (paddedlen > toklen)
+               return -EINVAL;
        *_tktlen = len;
 
        _debug("ticket len %u", len);
@@ -407,9 +412,8 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
                *_ticket = kmemdup(xdr, len, GFP_KERNEL);
                if (!*_ticket)
                        return -ENOMEM;
-               len = (len + 3) & ~3;
-               toklen -= len;
-               xdr += len >> 2;
+               toklen -= paddedlen;
+               xdr += paddedlen >> 2;
        }
 
        *_xdr = xdr;
@@ -552,7 +556,7 @@ static int rxrpc_preparse_xdr(struct key_preparsed_payload *prep)
 {
        const __be32 *xdr = prep->data, *token;
        const char *cp;
-       unsigned int len, tmp, loop, ntoken, toklen, sec_ix;
+       unsigned int len, paddedlen, loop, ntoken, toklen, sec_ix;
        size_t datalen = prep->datalen;
        int ret;
 
@@ -578,22 +582,21 @@ static int rxrpc_preparse_xdr(struct key_preparsed_payload *prep)
        if (len < 1 || len > AFSTOKEN_CELL_MAX)
                goto not_xdr;
        datalen -= 4;
-       tmp = (len + 3) & ~3;
-       if (tmp > datalen)
+       paddedlen = (len + 3) & ~3;
+       if (paddedlen > datalen)
                goto not_xdr;
 
        cp = (const char *) xdr;
        for (loop = 0; loop < len; loop++)
                if (!isprint(cp[loop]))
                        goto not_xdr;
-       if (len < tmp)
-               for (; loop < tmp; loop++)
-                       if (cp[loop])
-                               goto not_xdr;
+       for (; loop < paddedlen; loop++)
+               if (cp[loop])
+                       goto not_xdr;
        _debug("cellname: [%u/%u] '%*.*s'",
-              len, tmp, len, len, (const char *) xdr);
-       datalen -= tmp;
-       xdr += tmp >> 2;
+              len, paddedlen, len, len, (const char *) xdr);
+       datalen -= paddedlen;
+       xdr += paddedlen >> 2;
 
        /* get the token count */
        if (datalen < 12)
@@ -614,10 +617,11 @@ static int rxrpc_preparse_xdr(struct key_preparsed_payload *prep)
                sec_ix = ntohl(*xdr);
                datalen -= 4;
                _debug("token: [%x/%zx] %x", toklen, datalen, sec_ix);
-               if (toklen < 20 || toklen > datalen)
+               paddedlen = (toklen + 3) & ~3;
+               if (toklen < 20 || toklen > datalen || paddedlen > datalen)
                        goto not_xdr;
-               datalen -= (toklen + 3) & ~3;
-               xdr += (toklen + 3) >> 2;
+               datalen -= paddedlen;
+               xdr += paddedlen >> 2;
 
        } while (--loop > 0);