Skip to contentSkip to conversation

Prevent crashing when parsing bad X.25 facilities

PATCH Superseded Dan Rosenberg 2 participants 4 messages Nov 12, 2010

Subsystems (3)
Networking Network drivers X.25 stack

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Dan Rosenberg

author
#0 lore raw

PATCH v2 1/1 Prevent crashing when parsing bad X.25 facilities

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.

v2 prevents printing values outside the appropriate range.

Signed-off-by Dan Rosenberg <[email protected]>
CC stable <[email protected]>
---
 net/x25/x25_facilities.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)
net/x25/x25_facilities.c +10 -3
index 3a8c4c4..7f18e7d 100644
@@ -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,8 @@ 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;
maintainer
#1 lore raw

From: Dan Rosenberg <[email protected]>
Date: Fri, 12 Nov 2010 09:07:57 -0500

net/x25/x25_facilities.c int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, · line 162

Quoted 10 lines
net/x25/x25_facilities.c +2 -3
@@ -149,9 +157,8 @@ 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;

Thanks for not even compile testing your changes:

net/x25/x25_facilities.c: In function 'x25_parse_facilities':
net/x25/x25_facilities.c:161:6: error: expected ')' before 'p' net/x25/x25_facilities.c:161:6: warning: too few arguments for format

I find this kind of carelessness extremely amusing coming from someone who is so big on security theatre.

author
#2 lore raw
Quoted 3 lines

I find this kind of carelessness extremely amusing coming from someone who is so big on security theatre.

You know what I find amusing? The sheer number of security issues a single person can find in his spare time. Congratulations on your attention to detail.

-Dan

maintainer
#3 lore raw

From: Dan Rosenberg <[email protected]>
Date: Fri, 12 Nov 2010 15:44:25 -0500

Quoted 7 lines
Quoted 3 lines

I find this kind of carelessness extremely amusing coming from someone who is so big on security theatre.

You know what I find amusing? The sheer number of security issues a single person can find in his spare time. Congratulations on your attention to detail.

I do not even agree that, for example, pointer exposure is a real issue. It still remains a matter of opinion.

And much of your claims and boasting is based upon that opinion.

So don't pass it off saliently or indirectly as fact.