Patchwork x25: Prevent crashing when parsing bad X.25 facilities CVE-2010-4164

login
register
mail settings
Submitter Steve Conklin
Date March 8, 2011, 3:46 p.m.
Message ID <1299599171-9976-1-git-send-email-sconklin@canonical.com>
Download mbox | patch
Permalink /patch/86021/
State New
Headers show

Comments

Steve Conklin - March 8, 2011, 3:46 p.m.
From: Dan Rosenberg <drosenberg@vsecurity.com>

BugLink: http://bugs.launchpad.net/bugs/731199

CVE-2010-4164

Now with improved comma support.

On parsing malformed X.25 facilities, decrementing the remaining length
may cause it to underflow.  Since the length is an unsigned integer,
this will result in the loop continuing until the kernel crashes.

This patch adds checks to ensure decrementing the remaining length does
not cause it to wrap around.

Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
    (cherry picked from commit 5ef41308f94dcbb3b7afc56cdef1c2ba53fa5d2f)
Signed-off-by: Steve Conklin <sconklin@canonical.com>
---
 net/x25/x25_facilities.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
Tim Gardner - March 8, 2011, 4:24 p.m.
On 03/08/2011 03:46 PM, Steve Conklin wrote:
> From: Dan Rosenberg<drosenberg@vsecurity.com>
>
> BugLink: http://bugs.launchpad.net/bugs/731199
>
> CVE-2010-4164
>
> Now with improved comma support.
>
> On parsing malformed X.25 facilities, decrementing the remaining length
> may cause it to underflow.  Since the length is an unsigned integer,
> this will result in the loop continuing until the kernel crashes.
>
> This patch adds checks to ensure decrementing the remaining length does
> not cause it to wrap around.
>
> Signed-off-by: Dan Rosenberg<drosenberg@vsecurity.com>
> Signed-off-by: David S. Miller<davem@davemloft.net>
>      (cherry picked from commit 5ef41308f94dcbb3b7afc56cdef1c2ba53fa5d2f)
> Signed-off-by: Steve Conklin<sconklin@canonical.com>
> ---
>   net/x25/x25_facilities.c |   12 +++++++++---
>   1 files changed, 9 insertions(+), 3 deletions(-)
>

Acked-by: Tim Gardner <tim.gardner@canonical.com> for Hardy and Karmic

rtg
Steve Conklin - March 8, 2011, 5:08 p.m.
On Tue, 2011-03-08 at 16:24 +0000, Tim Gardner wrote:
> On 03/08/2011 03:46 PM, Steve Conklin wrote:
> > From: Dan Rosenberg<drosenberg@vsecurity.com>
> >
> > BugLink: http://bugs.launchpad.net/bugs/731199
> >
> > CVE-2010-4164
> >
> > Now with improved comma support.
> >
> > On parsing malformed X.25 facilities, decrementing the remaining length
> > may cause it to underflow.  Since the length is an unsigned integer,
> > this will result in the loop continuing until the kernel crashes.
> >
> > This patch adds checks to ensure decrementing the remaining length does
> > not cause it to wrap around.
> >
> > Signed-off-by: Dan Rosenberg<drosenberg@vsecurity.com>
> > Signed-off-by: David S. Miller<davem@davemloft.net>
> >      (cherry picked from commit 5ef41308f94dcbb3b7afc56cdef1c2ba53fa5d2f)
> > Signed-off-by: Steve Conklin<sconklin@canonical.com>
> > ---
> >   net/x25/x25_facilities.c |   12 +++++++++---
> >   1 files changed, 9 insertions(+), 3 deletions(-)
> >
> 
> Acked-by: Tim Gardner <tim.gardner@canonical.com> for Hardy and Karmic
> 
> rtg
> -- 
> Tim Gardner tim.gardner@canonical.com
> 
Applied to Hardy and Karmic

Patch

diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index 3a8c4c4..55187c8 100644
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -61,6 +61,8 @@  int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 	while (len > 0) {
 		switch (*p & X25_FAC_CLASS_MASK) {
 		case X25_FAC_CLASS_A:
+			if (len < 2)
+				return 0;
 			switch (*p) {
 			case X25_FAC_REVERSE:
 				if((p[1] & 0x81) == 0x81) {
@@ -104,6 +106,8 @@  int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			len -= 2;
 			break;
 		case X25_FAC_CLASS_B:
+			if (len < 3)
+				return 0;
 			switch (*p) {
 			case X25_FAC_PACKET_SIZE:
 				facilities->pacsize_in  = p[1];
@@ -125,6 +129,8 @@  int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			len -= 3;
 			break;
 		case X25_FAC_CLASS_C:
+			if (len < 4)
+				return 0;
 			printk(KERN_DEBUG "X.25: unknown facility %02X, "
 			       "values %02X, %02X, %02X\n",
 			       p[0], p[1], p[2], p[3]);
@@ -132,6 +138,8 @@  int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 			len -= 4;
 			break;
 		case X25_FAC_CLASS_D:
+			if (len < p[1] + 2)
+				return 0;
 			switch (*p) {
 			case X25_FAC_CALLING_AE:
 				if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
@@ -149,9 +157,7 @@  int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 				break;
 			default:
 				printk(KERN_DEBUG "X.25: unknown facility %02X,"
-					"length %d, values %02X, %02X, "
-					"%02X, %02X\n",
-					p[0], p[1], p[2], p[3], p[4], p[5]);
+					"length %d\n", p[0], p[1]);
 				break;
 			}
 			len -= p[1] + 2;