[Devel] [PATCH rh7 1/2] ms/netfilter: nf_tables: fix out-of-bounds in nft_chain_commit_update

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 31.08.2022 19:24, Konstantin Khorenko wrote:
> From: Taehee Yoo <ap420073 at gmail.com>
> 
> When chain name is changed, nft_chain_commit_update is called.
> In the nft_chain_commit_update, trans->ctx.chain->name has old chain name
> and nft_trans_chain_name(trans) has new chain name.
> If new chain name is longer than old chain name, KASAN warns
> slab-out-of-bounds.
> 
> [  175.015012] BUG: KASAN: slab-out-of-bounds in strcpy+0x9e/0xb0
> [  175.022735] Write of size 1 at addr ffff880114e022da by task iptables-compat/1458
> 
> [  175.031353] CPU: 0 PID: 1458 Comm: iptables-compat Not tainted 4.16.0-rc7+ #146
> [  175.031353] Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB, BIOS 5.6.5 07/08/2015
> [  175.031353] Call Trace:
> [  175.031353]  dump_stack+0x68/0xa0
> [  175.031353]  print_address_description+0xd0/0x260
> [  175.031353]  ? strcpy+0x9e/0xb0
> [  175.031353]  kasan_report+0x234/0x350
> [  175.031353]  __asan_report_store1_noabort+0x1c/0x20
> [  175.031353]  strcpy+0x9e/0xb0
> [  175.031353]  nf_tables_commit+0x1ccc/0x2990
> [  175.031353]  nfnetlink_rcv+0x141e/0x16c0
> [  175.031353]  ? nfnetlink_net_init+0x150/0x150
> [  175.031353]  ? lock_acquire+0x370/0x370
> [  175.031353]  ? lock_acquire+0x370/0x370
> [  175.031353]  netlink_unicast+0x444/0x640
> [  175.031353]  ? netlink_attachskb+0x700/0x700
> [  175.031353]  ? _copy_from_iter_full+0x180/0x740
> [  175.031353]  ? kasan_check_write+0x14/0x20
> [  175.031353]  ? _copy_from_user+0x9b/0xd0
> [  175.031353]  netlink_sendmsg+0x845/0xc70
> [ ... ]
> 
> Steps to reproduce:
>     iptables-compat -N 1
>     iptables-compat -E 1 aaaaaaaaa
> 
> Signed-off-by: Taehee Yoo <ap420073 at gmail.com>
> Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
> 
> (cherry picked from ms commit d71efb599ad42ef1e564c652d8084252bdc85edf)
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>   net/netfilter/nf_tables_api.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index b3840f07b82ca..a231a67d62c07 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -4068,7 +4068,7 @@ static void nft_chain_commit_update(struct nft_trans *trans)
>   	struct nft_base_chain *basechain;
>   
>   	if (nft_trans_chain_name(trans))
> -		strcpy(trans->ctx.chain->name, nft_trans_chain_name(trans));
> +		swap(trans->ctx.chain->name, nft_trans_chain_name(trans));
>   
>   	if (!(trans->ctx.chain->flags & NFT_BASE_CHAIN))
>   		return;

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.