[PATCH] tipc: Ensure NUL-termination of remote algorithm name

[PATCH] tipc: Ensure NUL-termination of remote algorithm name

[Dudu Lu]

In tipc_crypto_key_rcv(), the algorithm name is copied from the incoming
message using memcpy with a fixed size of TIPC_AEAD_ALG_NAME (32 bytes).
If the remote peer sends a name that fills all 32 bytes without a NUL
terminator, the alg_name field will not be NUL-terminated. This string
is later passed to crypto_alloc_aead() which expects a NUL-terminated
string, potentially causing an out-of-bounds read when the crypto
subsystem searches for the algorithm by name.

Fix by explicitly NUL-terminating the last byte of alg_name after the
memcpy.

Fixes: 1ef6f7c9390f ("tipc: add automatic session key exchange")
Signed-off-by: Dudu Lu <phx0fer@gmail.com>
---
 net/tipc/crypto.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c
index d3046a39ff72..ac072356bf0c 100644
--- a/net/tipc/crypto.c
+++ b/net/tipc/crypto.c
@@ -2325,6 +2325,7 @@ static bool tipc_crypto_key_rcv(struct tipc_crypto *rx, struct tipc_msg *hdr)
 	/* Copy key from msg data */
 	skey->keylen = keylen;
 	memcpy(skey->alg_name, data, TIPC_AEAD_ALG_NAME);
+	skey->alg_name[TIPC_AEAD_ALG_NAME - 1] = '\0';
 	memcpy(skey->key, data + TIPC_AEAD_ALG_NAME + sizeof(__be32),
 	       skey->keylen);
 
-- 
2.39.3 (Apple Git-145)

RE: [PATCH] tipc: Ensure NUL-termination of remote algorithm name

[Tung Quang Nguyen]

>Subject: [PATCH] tipc: Ensure NUL-termination of remote algorithm name
>
>In tipc_crypto_key_rcv(), the algorithm name is copied from the incoming
>message using memcpy with a fixed size of TIPC_AEAD_ALG_NAME (32 bytes).
>If the remote peer sends a name that fills all 32 bytes without a NUL
>terminator, the alg_name field will not be NUL-terminated. This string is later
>passed to crypto_alloc_aead() which expects a NUL-terminated string,
>potentially causing an out-of-bounds read when the crypto subsystem
>searches for the algorithm by name.
TIPC only supports one algorithm name "gcm(aes)" which is 8-byte length.
So, there is no " name that fills all 32 bytes" as you mentioned.
>
>Fix by explicitly NUL-terminating the last byte of alg_name after the memcpy.
>
>Fixes: 1ef6f7c9390f ("tipc: add automatic session key exchange")
>Signed-off-by: Dudu Lu <phx0fer@gmail.com>
>---
> net/tipc/crypto.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/net/tipc/crypto.c b/net/tipc/crypto.c index
>d3046a39ff72..ac072356bf0c 100644
>--- a/net/tipc/crypto.c
>+++ b/net/tipc/crypto.c
>@@ -2325,6 +2325,7 @@ static bool tipc_crypto_key_rcv(struct tipc_crypto
>*rx, struct tipc_msg *hdr)
> 	/* Copy key from msg data */
> 	skey->keylen = keylen;
> 	memcpy(skey->alg_name, data, TIPC_AEAD_ALG_NAME);
>+	skey->alg_name[TIPC_AEAD_ALG_NAME - 1] = '\0';
This is not needed as explained above.
> 	memcpy(skey->key, data + TIPC_AEAD_ALG_NAME + sizeof(__be32),
> 	       skey->keylen);
>
>--
>2.39.3 (Apple Git-145)
>