Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Aug 2023 01:20:34 +0200 (CEST)
From:   Jan Engelhardt <jengelh@inai.de>
To:     Justin Stitt <justinstitt@google.com>
cc:     Pablo Neira Ayuso <pablo@netfilter.org>,
        Jozsef Kadlecsik <kadlec@netfilter.org>,
        Florian Westphal <fw@strlen.de>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        linux-hardening@vger.kernel.org, Kees Cook <keescook@chromium.org>,
        netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 7/7] netfilter: xtables: refactor deprecated strncpy
In-Reply-To: <20230808-net-netfilter-v1-7-efbbe4ec60af@google.com>
Message-ID: <35rnr776-4ssp-314r-0473-p19q3r880ps1@vanv.qr>
References: <20230808-net-netfilter-v1-0-efbbe4ec60af@google.com> <20230808-net-netfilter-v1-7-efbbe4ec60af@google.com>
User-Agent: Alpine 2.26 (LSU 649 2022-06-02)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk


On Wednesday 2023-08-09 00:48, Justin Stitt wrote:

>Prefer `strscpy` as it's a more robust interface.
>
>There may have existed a bug here due to both `tbl->repl.name` and
>`info->name` having a size of 32 as defined below:
>|  #define XT_TABLE_MAXNAMELEN 32
>
>This may lead to buffer overreads in some situations -- `strscpy` solves
>this by guaranteeing NUL-termination of the dest buffer.

It generally will not lead to overreads.
xt not only deals with strings on its own turf, it even takes
them from userspace-provided buffers, which means extra scrutiny is
absolutely required. Done in places like

x_tables.c:     if (strnlen(name, XT_EXTENSION_MAXNAMELEN) == XT_EXTENSION_MAXNAMELEN)


(Which is not to say the strncpy->strscpy mop-up is bad.)