RE: [PATCH net] tipc: reject name table updates with invalid origin node

[Tung Quang Nguyen]

>Subject: [PATCH net] tipc: reject name table updates with invalid origin node
>
>From: Jun Yang <junvyyang@xxxxxxxxxxx>
>
>tipc_update_nametbl() trusts the origin node carried in a received
>NAME_DISTRIBUTOR message. For a WITHDRAWAL it removes and frees the
>matching publication with tipc_nametbl_remove_publ() (a {key,ref} wildcard
>match that ignores the node), then calls
>tipc_node_unsubscribe() to unlink the publication from the advertising node's
>publ_list.
>
>tipc_node_unsubscribe() is a no-op when in_own_node(net, node) is true,
>and in_own_node() treats node 0 as "own" (addr == own || !addr). So a
>WITHDRAWAL with orignode == 0 frees the publication via kfree_rcu() while
>the paired unsubscribe silently does nothing, leaving the freed publication
>linked on the owning peer's publ_list through its binding_node.
>
>A subsequent legitimate WITHDRAWAL for a neighbouring publication that the
>same peer advertised then performs list_del() across the freed object, writing
>a kernel pointer into freed memory (use-after-free
>write) and corrupting the live publ_list head. The whole sequence is attacker-
>driven from received packets and is reachable by an unprivileged local user,
>who can gain CAP_NET_ADMIN in a new network namespace via
>unshare(CLONE_NEWUSER|CLONE_NEWNET), enable a TIPC UDP bearer, and
>emulate a peer over loopback UDP.

Please do not create fake TIPC protocol message in insecure environment.
In insecure environment, IPSec needs to be used: https://datatracker.ietf.org/doc/html/draft-maloy-tipc-01.txt#section-6

>
>A name table update must always originate from a real peer node, never from
>our own address or from node 0. Reject such updates up front:
>in_own_node() already covers both cases, so a single guard at the top of
>tipc_update_nametbl() drops the malformed update before any publication is
>removed or freed.
>
>Fixes: 37922ea4a310 ("tipc: permit overlapping service ranges in name table")
>Cc: stable@xxxxxxxxxx
>Reported-by: TencentOS Corvus AI <corvus@xxxxxxxxxxx>
>Signed-off-by: Jun Yang <junvyyang@xxxxxxxxxxx>
>---
> net/tipc/name_distr.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c index
>ba4f4906e13b..a496e2e9ef62 100644
>--- a/net/tipc/name_distr.c
>+++ b/net/tipc/name_distr.c
>@@ -286,6 +286,9 @@ static bool tipc_update_nametbl(struct net *net, struct
>distr_item *i,
> 	u32 key = ntohl(i->key);
> 	struct tipc_uaddr ua;
>
>+	if (in_own_node(net, node))
>+		return false;

This is redundant because ' tipc_update_nametbl()' is called when receiving messages from other nodes.
 
>+
> 	/* A peer-advertised binding with lower > upper can never be
>matched
> 	 * or withdrawn and would leak the publication; the local bind path
> 	 * rejects such ranges, so reject ranges learned from the network too.
>--
>2.55.0
>