[SRU Z/Y/T] Fix CVE-2017-7482

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[SRU Z/Y/T] Fix CVE-2017-7482

Stefan Bader-2
Xenial already has this pending from upstream stable. For Zesty
and Yakkety the fix can be cherry-picked. For Trusty I had to
take the version from 4.4.y and fix up one hunk (which renamed
a stack variable) as it did not apply.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH Z/Y] rxrpc: Fix several cases where a padded len isn't checked in ticket decode

Stefan Bader-2
From: David Howells <[hidden email]>

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: 石磊 <[hidden email]>
Signed-off-by: David Howells <[hidden email]>
Reviewed-by: Marc Dionne <[hidden email]>
Reviewed-by: Dan Carpenter <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>

CVE-2017-7482

(cherry-picked from commit 5f2f97656ada8d811d3c1bef503ced266fcd53a0)
Signed-off-by: Stefan Bader <[hidden email]>
---
 net/rxrpc/key.c | 64 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 0a4e284..5436922 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -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);
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

Stefan Bader-2
In reply to this post by Stefan Bader-2
From: David Howells <[hidden email]>

commit 5f2f97656ada8d811d3c1bef503ced266fcd53a0 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: 石磊 <[hidden email]>
Signed-off-by: David Howells <[hidden email]>
Reviewed-by: Marc Dionne <[hidden email]>
Reviewed-by: Dan Carpenter <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
Signed-off-by: Greg Kroah-Hartman <[hidden email]>

CVE-2017-7482

(backported from commit eab38dfd66d7f13b9eecfae7728ff0d2e49ff16f 4.4.y)
Signed-off-by: Stefan Bader <[hidden email]>
---
 net/rxrpc/ar-key.c | 64 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/net/rxrpc/ar-key.c b/net/rxrpc/ar-key.c
index 7633a75..10e6e5d 100644
--- a/net/rxrpc/ar-key.c
+++ b/net/rxrpc/ar-key.c
@@ -213,7 +213,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 */
@@ -243,16 +243,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)
@@ -261,16 +261,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);
 
@@ -289,7 +289,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)
@@ -303,15 +303,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);
@@ -383,7 +385,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)
@@ -395,6 +397,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);
@@ -403,9 +408,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;
@@ -549,7 +553,7 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal
 {
  const __be32 *xdr = data, *token;
  const char *cp;
- unsigned int len, tmp, loop, ntoken, toklen, sec_ix;
+ unsigned int len, paddedlen, loop, ntoken, toklen, sec_ix;
  int ret;
 
  _enter(",{%x,%x,%x,%x},%zu",
@@ -574,22 +578,21 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal
  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)
@@ -610,10 +613,11 @@ static int rxrpc_instantiate_xdr(struct key *key, const void *data, size_t datal
  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);
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [SRU Z/Y/T] Fix CVE-2017-7482

Seth Forshee
In reply to this post by Stefan Bader-2
On Wed, Jul 12, 2017 at 04:30:24PM +0200, Stefan Bader wrote:
> Xenial already has this pending from upstream stable. For Zesty
> and Yakkety the fix can be cherry-picked. For Trusty I had to
> take the version from 4.4.y and fix up one hunk (which renamed
> a stack variable) as it did not apply.

For both versions:

Acked-by: Seth Forshee <[hidden email]>

Artful has also received this commit from upstream stable.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [SRU Z/Y/T] Fix CVE-2017-7482

Kleber Souza
In reply to this post by Stefan Bader-2
On 07/12/17 16:30, Stefan Bader wrote:
> Xenial already has this pending from upstream stable. For Zesty
> and Yakkety the fix can be cherry-picked. For Trusty I had to
> take the version from 4.4.y and fix up one hunk (which renamed
> a stack variable) as it did not apply.
>
> -Stefan
>
>

For both patches:

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED: [SRU Z/Y/T] Fix CVE-2017-7482

Thadeu Lima de Souza Cascardo-3
In reply to this post by Stefan Bader-2
Applied to trusty, yakkety and zesty master-next branches.

Thanks.
Cascardo.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Loading...