From: Oleh Konko 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 Signed-off-by: David Howells Reviewed-by: Jeffrey Altman cc: Marc Dionne cc: Eric Dumazet cc: "David S. Miller" cc: Jakub Kicinski cc: Paolo Abeni cc: Simon Horman 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 #include #include +#include #include #include #include @@ -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);