Commit 09c9faac authored by David Howells's avatar David Howells Committed by Ben Hutchings

rxrpc: Fix several cases where a padded len isn't checked in ticket decode

commit 5f2f9765 upstream.

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: default avatar石磊 <shilei-c@360.cn>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Reviewed-by: default avatarMarc Dionne <marc.c.dionne@auristor.com>
Reviewed-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
[bwh: Backported to 3.2: adjust filename, context]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent a7d51947
...@@ -213,7 +213,7 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ, ...@@ -213,7 +213,7 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
unsigned *_toklen) unsigned *_toklen)
{ {
const __be32 *xdr = *_xdr; const __be32 *xdr = *_xdr;
unsigned 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 /* there must be at least one name, and at least #names+1 length
* words */ * words */
...@@ -243,16 +243,16 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ, ...@@ -243,16 +243,16 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
toklen -= 4; toklen -= 4;
if (tmp <= 0 || tmp > AFSTOKEN_STRING_MAX) if (tmp <= 0 || tmp > AFSTOKEN_STRING_MAX)
return -EINVAL; return -EINVAL;
if (tmp > toklen) paddedlen = (tmp + 3) & ~3;
if (paddedlen > toklen)
return -EINVAL; return -EINVAL;
princ->name_parts[loop] = kmalloc(tmp + 1, GFP_KERNEL); princ->name_parts[loop] = kmalloc(tmp + 1, GFP_KERNEL);
if (!princ->name_parts[loop]) if (!princ->name_parts[loop])
return -ENOMEM; return -ENOMEM;
memcpy(princ->name_parts[loop], xdr, tmp); memcpy(princ->name_parts[loop], xdr, tmp);
princ->name_parts[loop][tmp] = 0; princ->name_parts[loop][tmp] = 0;
tmp = (tmp + 3) & ~3; toklen -= paddedlen;
toklen -= tmp; xdr += paddedlen >> 2;
xdr += tmp >> 2;
} }
if (toklen < 4) if (toklen < 4)
...@@ -261,16 +261,16 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ, ...@@ -261,16 +261,16 @@ static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
toklen -= 4; toklen -= 4;
if (tmp <= 0 || tmp > AFSTOKEN_K5_REALM_MAX) if (tmp <= 0 || tmp > AFSTOKEN_K5_REALM_MAX)
return -EINVAL; return -EINVAL;
if (tmp > toklen) paddedlen = (tmp + 3) & ~3;
if (paddedlen > toklen)
return -EINVAL; return -EINVAL;
princ->realm = kmalloc(tmp + 1, GFP_KERNEL); princ->realm = kmalloc(tmp + 1, GFP_KERNEL);
if (!princ->realm) if (!princ->realm)
return -ENOMEM; return -ENOMEM;
memcpy(princ->realm, xdr, tmp); memcpy(princ->realm, xdr, tmp);
princ->realm[tmp] = 0; princ->realm[tmp] = 0;
tmp = (tmp + 3) & ~3; toklen -= paddedlen;
toklen -= tmp; xdr += paddedlen >> 2;
xdr += tmp >> 2;
_debug("%s/...@%s", princ->name_parts[0], princ->realm); _debug("%s/...@%s", princ->name_parts[0], princ->realm);
...@@ -289,7 +289,7 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td, ...@@ -289,7 +289,7 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td,
unsigned *_toklen) unsigned *_toklen)
{ {
const __be32 *xdr = *_xdr; const __be32 *xdr = *_xdr;
unsigned toklen = *_toklen, len; unsigned int toklen = *_toklen, len, paddedlen;
/* there must be at least one tag and one length word */ /* there must be at least one tag and one length word */
if (toklen <= 8) if (toklen <= 8)
...@@ -303,6 +303,9 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td, ...@@ -303,6 +303,9 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td,
toklen -= 8; toklen -= 8;
if (len > max_data_size) if (len > max_data_size)
return -EINVAL; return -EINVAL;
paddedlen = (len + 3) & ~3;
if (paddedlen > toklen)
return -EINVAL;
td->data_len = len; td->data_len = len;
if (len > 0) { if (len > 0) {
...@@ -310,9 +313,8 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td, ...@@ -310,9 +313,8 @@ static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td,
if (!td->data) if (!td->data)
return -ENOMEM; return -ENOMEM;
memcpy(td->data, xdr, len); memcpy(td->data, xdr, len);
len = (len + 3) & ~3; toklen -= paddedlen;
toklen -= len; xdr += paddedlen >> 2;
xdr += len >> 2;
} }
_debug("tag %x len %x", td->tag, td->data_len); _debug("tag %x len %x", td->tag, td->data_len);
...@@ -384,7 +386,7 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen, ...@@ -384,7 +386,7 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
const __be32 **_xdr, unsigned *_toklen) const __be32 **_xdr, unsigned *_toklen)
{ {
const __be32 *xdr = *_xdr; const __be32 *xdr = *_xdr;
unsigned toklen = *_toklen, len; unsigned int toklen = *_toklen, len, paddedlen;
/* there must be at least one length word */ /* there must be at least one length word */
if (toklen <= 4) if (toklen <= 4)
...@@ -396,6 +398,9 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen, ...@@ -396,6 +398,9 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
toklen -= 4; toklen -= 4;
if (len > AFSTOKEN_K5_TIX_MAX) if (len > AFSTOKEN_K5_TIX_MAX)
return -EINVAL; return -EINVAL;
paddedlen = (len + 3) & ~3;
if (paddedlen > toklen)
return -EINVAL;
*_tktlen = len; *_tktlen = len;
_debug("ticket len %u", len); _debug("ticket len %u", len);
...@@ -405,9 +410,8 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen, ...@@ -405,9 +410,8 @@ static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
if (!*_ticket) if (!*_ticket)
return -ENOMEM; return -ENOMEM;
memcpy(*_ticket, xdr, len); memcpy(*_ticket, xdr, len);
len = (len + 3) & ~3; toklen -= paddedlen;
toklen -= len; xdr += paddedlen >> 2;
xdr += len >> 2;
} }
*_xdr = xdr; *_xdr = xdr;
...@@ -551,7 +555,7 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal ...@@ -551,7 +555,7 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal
{ {
const __be32 *xdr = data, *token; const __be32 *xdr = data, *token;
const char *cp; const char *cp;
unsigned len, tmp, loop, ntoken, toklen, sec_ix; unsigned int len, paddedlen, loop, ntoken, toklen, sec_ix;
int ret; int ret;
_enter(",{%x,%x,%x,%x},%zu", _enter(",{%x,%x,%x,%x},%zu",
...@@ -576,22 +580,21 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal ...@@ -576,22 +580,21 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal
if (len < 1 || len > AFSTOKEN_CELL_MAX) if (len < 1 || len > AFSTOKEN_CELL_MAX)
goto not_xdr; goto not_xdr;
datalen -= 4; datalen -= 4;
tmp = (len + 3) & ~3; paddedlen = (len + 3) & ~3;
if (tmp > datalen) if (paddedlen > datalen)
goto not_xdr; goto not_xdr;
cp = (const char *) xdr; cp = (const char *) xdr;
for (loop = 0; loop < len; loop++) for (loop = 0; loop < len; loop++)
if (!isprint(cp[loop])) if (!isprint(cp[loop]))
goto not_xdr; goto not_xdr;
if (len < tmp) for (; loop < paddedlen; loop++)
for (; loop < tmp; loop++) if (cp[loop])
if (cp[loop]) goto not_xdr;
goto not_xdr;
_debug("cellname: [%u/%u] '%*.*s'", _debug("cellname: [%u/%u] '%*.*s'",
len, tmp, len, len, (const char *) xdr); len, paddedlen, len, len, (const char *) xdr);
datalen -= tmp; datalen -= paddedlen;
xdr += tmp >> 2; xdr += paddedlen >> 2;
/* get the token count */ /* get the token count */
if (datalen < 12) if (datalen < 12)
...@@ -612,10 +615,11 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal ...@@ -612,10 +615,11 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal
sec_ix = ntohl(*xdr); sec_ix = ntohl(*xdr);
datalen -= 4; datalen -= 4;
_debug("token: [%x/%zx] %x", toklen, datalen, sec_ix); _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; goto not_xdr;
datalen -= (toklen + 3) & ~3; datalen -= paddedlen;
xdr += (toklen + 3) >> 2; xdr += paddedlen >> 2;
} while (--loop > 0); } while (--loop > 0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment