From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752143Ab1K2Hdz (ORCPT ); Tue, 29 Nov 2011 02:33:55 -0500 Received: from mail-yx0-f174.google.com ([209.85.213.174]:45941 "EHLO mail-yx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751099Ab1K2Hdx convert rfc822-to-8bit (ORCPT ); Tue, 29 Nov 2011 02:33:53 -0500 Subject: Re: [PATCH] sctp: integer overflow in sctp_auth_create_key() Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Xi Wang In-Reply-To: <4ED3AC7D.6090108@hp.com> Date: Tue, 29 Nov 2011 02:33:50 -0500 Cc: linux-kernel@vger.kernel.org, Sridhar Samudrala , "David S. Miller" , linux-sctp@vger.kernel.org, netdev@vger.kernel.org, security@kernel.org Content-Transfer-Encoding: 8BIT Message-Id: <147F953A-CC69-41FF-ACD4-64E5E2956411@gmail.com> References: <426D7BA8-ECD0-44D6-A09F-2033F0C825FC@gmail.com> <4ED3AC7D.6090108@hp.com> To: Vladislav Yasevich X-Mailer: Apple Mail (2.1084) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > > 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