* [PATCH] Fix out-of-bounds reading in sctp_asoc_get_hmac()
@ 2010-10-01 21:51 Dan Rosenberg
2010-10-01 22:13 ` Vlad Yasevich
0 siblings, 1 reply; 3+ messages in thread
From: Dan Rosenberg @ 2010-10-01 21:51 UTC (permalink / raw)
To: vladislav.yasevich, sri
Cc: linux-sctp, linux-kernel, security, stable, netdev
The sctp_asoc_get_hmac() function iterates through a peer's hmac_ids
array and attempts to ensure that only a supported hmac entry is
returned. The current code fails to do this properly - if the last id
in the array is out of range (greater than SCTP_AUTH_HMAC_ID_MAX), the
id integer remains set after exiting the loop, and the address of an
out-of-bounds entry will be returned and subsequently used in the parent
function, causing potentially ugly memory corruption. This patch resets
the id integer to 0 on encountering an invalid id so that NULL will be
returned after finishing the loop if no valid ids are found.
Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
--- linux-2.6.35.5.orig/net/sctp/auth.c 2010-09-20 16:59:09.000000000 -0400
+++ linux-2.6.35.5/net/sctp/auth.c 2010-10-01 16:48:58.000000000 -0400
@@ -543,16 +543,20 @@ struct sctp_hmac *sctp_auth_asoc_get_hma
id = ntohs(hmacs->hmac_ids[i]);
/* Check the id is in the supported range */
- if (id > SCTP_AUTH_HMAC_ID_MAX)
+ if (id > SCTP_AUTH_HMAC_ID_MAX) {
+ id = 0;
continue;
+ }
/* See is we support the id. Supported IDs have name and
* length fields set, so that we can allocated and use
* them. We can safely just check for name, for without the
* name, we can't allocate the TFM.
*/
- if (!sctp_hmac_list[id].hmac_name)
+ if (!sctp_hmac_list[id].hmac_name) {
+ id = 0;
continue;
+ }
break;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix out-of-bounds reading in sctp_asoc_get_hmac()
2010-10-01 21:51 [PATCH] Fix out-of-bounds reading in sctp_asoc_get_hmac() Dan Rosenberg
@ 2010-10-01 22:13 ` Vlad Yasevich
2010-10-04 5:00 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Vlad Yasevich @ 2010-10-01 22:13 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: sri, linux-sctp, linux-kernel, security, stable, netdev
On 10/01/2010 05:51 PM, Dan Rosenberg wrote:
> The sctp_asoc_get_hmac() function iterates through a peer's hmac_ids
> array and attempts to ensure that only a supported hmac entry is
> returned. The current code fails to do this properly - if the last id
> in the array is out of range (greater than SCTP_AUTH_HMAC_ID_MAX), the
> id integer remains set after exiting the loop, and the address of an
> out-of-bounds entry will be returned and subsequently used in the parent
> function, causing potentially ugly memory corruption. This patch resets
> the id integer to 0 on encountering an invalid id so that NULL will be
> returned after finishing the loop if no valid ids are found.
>
> Signed-off-by: Dan Rosenberg<drosenberg@vsecurity.com>
Good catch.
Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>
-vlad
>
> --- linux-2.6.35.5.orig/net/sctp/auth.c 2010-09-20 16:59:09.000000000 -0400
> +++ linux-2.6.35.5/net/sctp/auth.c 2010-10-01 16:48:58.000000000 -0400
> @@ -543,16 +543,20 @@ struct sctp_hmac *sctp_auth_asoc_get_hma
> id = ntohs(hmacs->hmac_ids[i]);
>
> /* Check the id is in the supported range */
> - if (id> SCTP_AUTH_HMAC_ID_MAX)
> + if (id> SCTP_AUTH_HMAC_ID_MAX) {
> + id = 0;
> continue;
> + }
>
> /* See is we support the id. Supported IDs have name and
> * length fields set, so that we can allocated and use
> * them. We can safely just check for name, for without the
> * name, we can't allocate the TFM.
> */
> - if (!sctp_hmac_list[id].hmac_name)
> + if (!sctp_hmac_list[id].hmac_name) {
> + id = 0;
> continue;
> + }
>
> break;
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix out-of-bounds reading in sctp_asoc_get_hmac()
2010-10-01 22:13 ` Vlad Yasevich
@ 2010-10-04 5:00 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2010-10-04 5:00 UTC (permalink / raw)
To: vladislav.yasevich
Cc: drosenberg, sri, linux-sctp, linux-kernel, security, stable,
netdev
From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Fri, 01 Oct 2010 18:13:34 -0400
> On 10/01/2010 05:51 PM, Dan Rosenberg wrote:
>> The sctp_asoc_get_hmac() function iterates through a peer's hmac_ids
>> array and attempts to ensure that only a supported hmac entry is
>> returned. The current code fails to do this properly - if the last id
>> in the array is out of range (greater than SCTP_AUTH_HMAC_ID_MAX), the
>> id integer remains set after exiting the loop, and the address of an
>> out-of-bounds entry will be returned and subsequently used in the
>> parent
>> function, causing potentially ugly memory corruption. This patch
>> resets
>> the id integer to 0 on encountering an invalid id so that NULL will be
>> returned after finishing the loop if no valid ids are found.
>>
>> Signed-off-by: Dan Rosenberg<drosenberg@vsecurity.com>
>
> Good catch.
>
> Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>
Applied.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-04 5:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-01 21:51 [PATCH] Fix out-of-bounds reading in sctp_asoc_get_hmac() Dan Rosenberg
2010-10-01 22:13 ` Vlad Yasevich
2010-10-04 5:00 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).