linux-kernel - Re: [PATCH 5.10 68/77] sctp: add vtag check in sctp_sf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a3059f52-54c6-6ab3-0f1d-a9b1566ff118@ispras.ru>
Date:   Tue, 2 Nov 2021 17:12:16 +0300
From:   Alexey Khoroshilov <khoroshilov@...ras.ru>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Cc:     stable@...r.kernel.org, Xin Long <lucien.xin@...il.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Sasha Levin <sashal@...nel.org>, ldv-project@...uxtesting.org
Subject: Re: [PATCH 5.10 68/77] sctp: add vtag check in sctp_sf_violation

Hello!

It seems the patch may lead to NULL pointer dereference.


1. sctp_sf_violation_chunk() calls sctp_sf_violation() with asoc arg
equal to NULL.

static enum sctp_disposition sctp_sf_violation_chunk(
...
{
...
    if (!asoc)
        return sctp_sf_violation(net, ep, asoc, type, arg, commands);
...

2. Newly added code of sctp_sf_violation() calls to sctp_vtag_verify()
with asoc arg equal to NULL.

enum sctp_disposition sctp_sf_violation(struct net *net,
...
{
    struct sctp_chunk *chunk = arg;

    if (!sctp_vtag_verify(chunk, asoc))
        return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
...

3. sctp_vtag_verify() dereferences asoc without any check.

/* Check VTAG of the packet matches the sender's own tag. */
static inline int
sctp_vtag_verify(const struct sctp_chunk *chunk,
		 const struct sctp_association *asoc)
{
	/* RFC 2960 Sec 8.5 When receiving an SCTP packet, the endpoint
	 * MUST ensure that the value in the Verification Tag field of
	 * the received SCTP packet matches its own Tag. If the received
	 * Verification Tag value does not match the receiver's own
	 * tag value, the receiver shall silently discard the packet...
	 */
	if (ntohl(chunk->sctp_hdr->vtag) != asoc->c.my_vtag)
		return 0;


Found by Linux Verification Center (linuxtesting.org) with SVACE tool.

--
Best regards,
Alexey Khoroshilov
Linux Verification Center, ISPRAS


On 01.11.2021 12:17, Greg Kroah-Hartman wrote:
> From: Xin Long <lucien.xin@...il.com>
> 
> [ Upstream commit aa0f697e45286a6b5f0ceca9418acf54b9099d99 ]
> 
> sctp_sf_violation() is called when processing HEARTBEAT_ACK chunk
> in cookie_wait state, and some other places are also using it.
> 
> The vtag in the chunk's sctphdr should be verified, otherwise, as
> later in chunk length check, it may send abort with the existent
> asoc's vtag, which can be exploited by one to cook a malicious
> chunk to terminate a SCTP asoc.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Xin Long <lucien.xin@...il.com>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  net/sctp/sm_statefuns.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 0cfbf6046bf8..324c0222d9e6 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4549,6 +4549,9 @@ enum sctp_disposition sctp_sf_violation(struct net *net,
>  {
>  	struct sctp_chunk *chunk = arg;
>  
> +	if (!sctp_vtag_verify(chunk, asoc))
> +		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
> +
>  	/* Make sure that the chunk has a valid length. */
>  	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
>  		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
>