netdev - Re: [PATCH] sctp: integer overflow in sctp_auth_create

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 for Android: free password hash cracker in your pocket

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

Message-Id: <147F953A-CC69-41FF-ACD4-64E5E2956411@gmail.com>
Date:	Tue, 29 Nov 2011 02:33:50 -0500
From:	Xi Wang <xi.wang@...il.com>
To:	Vladislav Yasevich <vladislav.yasevich@...com>
Cc:	linux-kernel@...r.kernel.org, Sridhar Samudrala <sri@...ibm.com>,
	"David S. Miller" <davem@...emloft.net>,
	linux-sctp@...r.kernel.org, netdev@...r.kernel.org,
	security@...nel.org
Subject: Re: [PATCH] sctp: integer overflow in sctp_auth_create_key()

I agree that this is not a security issue if key_len can never get large.

So how about just removing the overflow check at all?

- xi

On Nov 28, 2011, at 10:45 AM, Vladislav Yasevich wrote:
> 
> Hmm.  Yes, this is a more correct check.
> 
> Acked-by: Vlad Yasevich <vladislav.yasevich@...com>
> 
> 
> However, I don't think this is a security issue.  As I've written before, this function is
> called from 2 places:
> 
>  1) setsockopt() code path
> 
>  2) sctp_auth_asoc_set_secret() code path
> 
> In case (1), sca_keylength is never going to exceed 65535 since it's
> bounded by a u16 from the user api.  As such, The integer promotion will
> not impact anything and the malloc() will never overflow.
> 
> In case (2), sca_keylength is computed based on the key the user provided
> (MAX_USHORT) and the combination of protocol negotiated data where that
> combination has a max size of 3 * MAX_USHORT (see sctp_auth_make_key_vector()).
> So, even this case, our maximum key length can be 4* MAX_USHORT which still
> will always be below MAX_INT and will not overflow.
> 
> So, I don't think there is big security consideration here, just a bad
> check that just happens to always work.
> 
> -vlad

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html