From patchwork Sat Mar 19 23:28:33 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Rosenberg X-Patchwork-Id: 87641 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 4E89BB6F14 for ; Sun, 20 Mar 2011 10:35:25 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751161Ab1CSXfU (ORCPT ); Sat, 19 Mar 2011 19:35:20 -0400 Received: from mx1.vsecurity.com ([209.67.252.12]:51319 "EHLO mx1.vsecurity.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751098Ab1CSXfS (ORCPT ); Sat, 19 Mar 2011 19:35:18 -0400 X-Greylist: delayed 399 seconds by postgrey-1.27 at vger.kernel.org; Sat, 19 Mar 2011 19:35:18 EDT Received: (qmail 19059 invoked from network); 19 Mar 2011 23:28:36 -0000 Received: from inbound.cs.brown.edu (HELO [127.0.0.1]) (drosenbe@[128.148.33.14]) (envelope-sender ) by mx1.vsecurity.com (qmail-ldap-1.03) with SMTP for ; 19 Mar 2011 23:28:36 -0000 Subject: [PATCH] ROSE: prevent heap corruption with bad facilities From: Dan Rosenberg To: ralf@linux-mips.org, davem@davemloft.net Cc: netdev@vger.kernel.org, security@kernel.org Date: Sat, 19 Mar 2011 19:28:33 -0400 Message-ID: <1300577313.12507.17.camel@dan> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When parsing the FAC_NATIONAL_DIGIS facilities field, keeping the counter field in an unsigned char is insufficient, since the counter is incremented by AX25_ADDR_LEN (7) with each loop. Providing a field length of 0xf, for example, causes heap corruption because the counter wraps without ever reaching the length and data is copied past the boundaries of the source_digis or dest_digis facilities arrays. Change the counter to an unsigned ints to prevent this wrapping and overflow. Additionally, when parsing the FAC_CCITT_DEST_NSAP and FAC_CCITT_SRC_NSAP facilities fields, a length of less than 10 results in an underflow in a memcpy size, resulting in a kernel panic due to massive heap corruption. Abort facilities parsing on this invalid length value. Signed-off-by: Dan Rosenberg Cc: stable@kernel.org --- net/rose/rose_subr.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/rose/rose_subr.c b/net/rose/rose_subr.c index 1734abb..fee9de4 100644 --- a/net/rose/rose_subr.c +++ b/net/rose/rose_subr.c @@ -240,7 +240,8 @@ int rose_decode(struct sk_buff *skb, int *ns, int *nr, int *q, int *d, int *m) static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *facilities, int len) { unsigned char *pt; - unsigned char l, lg, n = 0; + unsigned char l, n = 0; + unsigned int lg; int fac_national_digis_received = 0; do { @@ -334,12 +335,16 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac case 0xC0: l = p[1]; if (*p == FAC_CCITT_DEST_NSAP) { + if (l < 10) + return -1; memcpy(&facilities->source_addr, p + 7, ROSE_ADDR_LEN); memcpy(callsign, p + 12, l - 10); callsign[l - 10] = '\0'; asc2ax(&facilities->source_call, callsign); } if (*p == FAC_CCITT_SRC_NSAP) { + if (l < 10) + return -1; memcpy(&facilities->dest_addr, p + 7, ROSE_ADDR_LEN); memcpy(callsign, p + 12, l - 10); callsign[l - 10] = '\0'; @@ -379,6 +384,11 @@ int rose_parse_facilities(unsigned char *p, case FAC_CCITT: /* CCITT */ len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1); + + /* Invalid facilities */ + if (len < 0) + return 0; + facilities_len -= len + 1; p += len + 1; break;