[PATCH net] udp6: fix potential access to stale information

[PATCH net] udp6: fix potential access to stale information

[Eric Dumazet]

lena wang reported an issue caused by udpv6_sendmsg()
mangling msg->msg_name and msg->msg_namelen, which
are later read from ____sys_sendmsg() :

	/*
	 * If this is sendmmsg() and sending to current destination address was
	 * successful, remember it.
	 */
	if (used_address && err >= 0) {
		used_address->name_len = msg_sys->msg_namelen;
		if (msg_sys->msg_name)
			memcpy(&used_address->name, msg_sys->msg_name,
			       used_address->name_len);
	}

udpv6_sendmsg() wants to pretend the remote address family
is AF_INET in order to call udp_sendmsg().

A fix would be to modify the address in-place, instead
of using a local variable, but this could have other side effects.

Instead, restore initial values before we return from udpv6_sendmsg().

Fixes: c71d8ebe7a44 ("net: Fix security_socket_sendmsg() bypass problem.")
Reported-by: lena wang <lena.wang@mediatek.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/udp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9fb2f33ee3a76a09bbe15a9aaf1371a804f91ee2..a675acfb901d102ce56563b1d50ae827d9e04859 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1395,9 +1395,11 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 			msg->msg_name = &sin;
 			msg->msg_namelen = sizeof(sin);
 do_udp_sendmsg:
-			if (ipv6_only_sock(sk))
-				return -ENETUNREACH;
-			return udp_sendmsg(sk, msg, len);
+			err = ipv6_only_sock(sk) ?
+				-ENETUNREACH : udp_sendmsg(sk, msg, len);
+			msg->msg_name = sin6;
+			msg->msg_namelen = addr_len;
+			return err;
 		}
 	}
 
-- 
2.40.0.577.gac1e443424-goog

Re: [PATCH net] udp6: fix potential access to stale information

[Maciej Żenczykowski]

On Wed, Apr 12, 2023 at 3:03 PM Eric Dumazet <edumazet@google.com> wrote:
>
> lena wang reported an issue caused by udpv6_sendmsg()
> mangling msg->msg_name and msg->msg_namelen, which
> are later read from ____sys_sendmsg() :
>
>         /*
>          * If this is sendmmsg() and sending to current destination address was
>          * successful, remember it.
>          */
>         if (used_address && err >= 0) {
>                 used_address->name_len = msg_sys->msg_namelen;
>                 if (msg_sys->msg_name)
>                         memcpy(&used_address->name, msg_sys->msg_name,
>                                used_address->name_len);
>         }
>
> udpv6_sendmsg() wants to pretend the remote address family
> is AF_INET in order to call udp_sendmsg().
>
> A fix would be to modify the address in-place, instead
> of using a local variable, but this could have other side effects.
>
> Instead, restore initial values before we return from udpv6_sendmsg().
>
> Fixes: c71d8ebe7a44 ("net: Fix security_socket_sendmsg() bypass problem.")
> Reported-by: lena wang <lena.wang@mediatek.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Maciej Żenczykowski <maze@google.com>
> ---
>  net/ipv6/udp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 9fb2f33ee3a76a09bbe15a9aaf1371a804f91ee2..a675acfb901d102ce56563b1d50ae827d9e04859 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1395,9 +1395,11 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>                         msg->msg_name = &sin;
>                         msg->msg_namelen = sizeof(sin);
>  do_udp_sendmsg:
> -                       if (ipv6_only_sock(sk))
> -                               return -ENETUNREACH;
> -                       return udp_sendmsg(sk, msg, len);
> +                       err = ipv6_only_sock(sk) ?
> +                               -ENETUNREACH : udp_sendmsg(sk, msg, len);
> +                       msg->msg_name = sin6;
> +                       msg->msg_namelen = addr_len;
> +                       return err;
>                 }
>         }
>
> --
> 2.40.0.577.gac1e443424-goog

Reviewed-by: Maciej Żenczykowski <maze@google.com>

Re: [PATCH net] udp6: fix potential access to stale information

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 12 Apr 2023 13:03:08 +0000 you wrote:
> lena wang reported an issue caused by udpv6_sendmsg()
> mangling msg->msg_name and msg->msg_namelen, which
> are later read from ____sys_sendmsg() :
> 
> 	/*
> 	 * If this is sendmmsg() and sending to current destination address was
> 	 * successful, remember it.
> 	 */
> 	if (used_address && err >= 0) {
> 		used_address->name_len = msg_sys->msg_namelen;
> 		if (msg_sys->msg_name)
> 			memcpy(&used_address->name, msg_sys->msg_name,
> 			       used_address->name_len);
> 	}
> 
> [...]

Here is the summary with links:
  - [net] udp6: fix potential access to stale information
    https://git.kernel.org/netdev/net/c/1c5950fc6fe9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html