From: Oleh Konko <security@1seal.org>
rxrpc_preparse_xdr_yfs_rxgk() reads the raw key length and ticket length
from the XDR token as u32 values and passes each through round_up(x, 4)
before using the rounded value for validation and allocation. When the raw
length is >= 0xfffffffd, round_up() wraps to 0, so the bounds check and
kzalloc both use 0 while the subsequent memcpy still copies the original
~4 GiB value, producing a heap buffer overflow reachable from an
unprivileged add_key() call.
Fix this by:
(1) Rejecting raw key lengths above AFSTOKEN_GK_KEY_MAX and raw ticket
lengths above AFSTOKEN_GK_TOKEN_MAX before rounding, consistent with
the caps that the RxKAD path already enforces via AFSTOKEN_RK_TIX_MAX.
(2) Sizing the flexible-array allocation from the validated raw key
length via struct_size_t() instead of the rounded value.
(3) Caching the raw lengths so that the later field assignments and
memcpy calls do not re-read from the token, eliminating a class of
TOCTOU re-parse.
The control path (valid token with lengths within bounds) is unaffected.
Fixes: 0ca100ff4df6 ("rxrpc: Add YFS RxGK (GSSAPI) security class")
Signed-off-by: Oleh Konko <security@1seal.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
cc: stable@kernel.org
---
net/rxrpc/key.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 26d4336a4a02..77237a82be3b 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -13,6 +13,7 @@
#include <crypto/skcipher.h>
#include <linux/module.h>
#include <linux/net.h>
+#include <linux/overflow.h>
#include <linux/skbuff.h>
#include <linux/key-type.h>
#include <linux/ctype.h>
@@ -171,7 +172,7 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
size_t plen;
const __be32 *ticket, *key;
s64 tmp;
- u32 tktlen, keylen;
+ size_t raw_keylen, raw_tktlen, keylen, tktlen;
_enter(",{%x,%x,%x,%x},%x",
ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),
@@ -181,18 +182,22 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
goto reject;
key = xdr + (6 * 2 + 1);
- keylen = ntohl(key[-1]);
- _debug("keylen: %x", keylen);
- keylen = round_up(keylen, 4);
+ raw_keylen = ntohl(key[-1]);
+ _debug("keylen: %zx", raw_keylen);
+ if (raw_keylen > AFSTOKEN_GK_KEY_MAX)
+ goto reject;
+ keylen = round_up(raw_keylen, 4);
if ((6 * 2 + 2) * 4 + keylen > toklen)
goto reject;
ticket = xdr + (6 * 2 + 1 + (keylen / 4) + 1);
- tktlen = ntohl(ticket[-1]);
- _debug("tktlen: %x", tktlen);
- tktlen = round_up(tktlen, 4);
+ raw_tktlen = ntohl(ticket[-1]);
+ _debug("tktlen: %zx", raw_tktlen);
+ if (raw_tktlen > AFSTOKEN_GK_TOKEN_MAX)
+ goto reject;
+ tktlen = round_up(raw_tktlen, 4);
if ((6 * 2 + 2) * 4 + keylen + tktlen != toklen) {
- kleave(" = -EKEYREJECTED [%x!=%x, %x,%x]",
+ kleave(" = -EKEYREJECTED [%zx!=%x, %zx,%zx]",
(6 * 2 + 2) * 4 + keylen + tktlen, toklen,
keylen, tktlen);
goto reject;
@@ -206,7 +211,7 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
if (!token)
goto nomem;
- token->rxgk = kzalloc(sizeof(*token->rxgk) + keylen, GFP_KERNEL);
+ token->rxgk = kzalloc(struct_size_t(struct rxgk_key, _key, raw_keylen), GFP_KERNEL);
if (!token->rxgk)
goto nomem_token;
@@ -221,9 +226,9 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
token->rxgk->enctype = tmp = xdr_dec64(xdr + 5 * 2);
if (tmp < 0 || tmp > UINT_MAX)
goto reject_token;
- token->rxgk->key.len = ntohl(key[-1]);
+ token->rxgk->key.len = raw_keylen;
token->rxgk->key.data = token->rxgk->_key;
- token->rxgk->ticket.len = ntohl(ticket[-1]);
+ token->rxgk->ticket.len = raw_tktlen;
if (token->rxgk->endtime != 0) {
expiry = rxrpc_s64_to_time64(token->rxgk->endtime);
@@ -236,8 +241,7 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
memcpy(token->rxgk->key.data, key, token->rxgk->key.len);
/* Pad the ticket so that we can use it directly in XDR */
- token->rxgk->ticket.data = kzalloc(round_up(token->rxgk->ticket.len, 4),
- GFP_KERNEL);
+ token->rxgk->ticket.data = kzalloc(tktlen, GFP_KERNEL);
if (!token->rxgk->ticket.data)
goto nomem_yrxgk;
memcpy(token->rxgk->ticket.data, ticket, token->rxgk->ticket.len);