Prevent crashing when parsing bad X.25 facilities
Subsystems (3)
PATCH 1/1 Prevent crashing when parsing bad X.25 facilities
Sorry I didn't catch this at the same time as my previous report so it could be included in one patch.
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.
--- net/x25/x25_facilities.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
net/x25/x25_facilities.c +8 -0
@@ -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 )
Tested ok, although I noticed the CLASS_D default: message tries to print the facility type, length and 4 values, while it may only have 2 values. Just needs p[4] and p[5] taken out. Do you want me to spin a secondary patch or do you want to put them together?
Quoted 59 lines · Dan Rosenberg
Sorry I didn't catch this at the same time as my previous report so it could be included in one patch.
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 <[email protected]>CC stable <[email protected]>---
net/x25/x25_facilities.c | 8 ++++++++1 files changed, 8 insertions(+), 0 deletions(-)net/x25/x25_facilities.c +0
-0@@ -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) {
caseX25_FAC_REVERSE:
if((p[1] &0x81) ==0x81) {net/x25/x25_facilities.c +0
-0@@ -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) {
caseX25_FAC_PACKET_SIZE:
facilities->pacsize_in = p[1];net/x25/x25_facilities.c +0
-0@@ -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]);net/x25/x25_facilities.c +0
-0@@ -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) {
caseX25_FAC_CALLING_AE:
if (p[1] >X25_MAX_DTE_FACIL_LEN|| p[1] <= 1)
I can take care of it. Do you want me to just remove those items from the printk statement?
-Dan
Quoted 67 lines · Andrew Hendry
Tested ok, although I noticed the
CLASS_Ddefault: message tries to print the facility type, length and 4 values, while it may only have 2 values. Just needs p[4] and p[5] taken out. Do you want me to spin a secondary patch or do you want to put them together?Acked-by Andrew Hendry <[email protected]>Quoted 59 lines · Dan Rosenberg
Sorry I didn't catch this at the same time as my previous report so it could be included in one patch.
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 <[email protected]>CC stable <[email protected]>--- net/x25/x25_facilities.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)net/x25/x25_facilities.c +8
-0@@ -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 )
Thanks, yes just remove them from the printk.
Quoted 76 lines · Dan Rosenberg
I can take care of it. Do you want me to just remove those items from the printk statement?
-Dan
Quoted 67 lines · Andrew Hendry
Tested ok, although I noticed the
CLASS_Ddefault: message tries to print the facility type, length and 4 values, while it may only have 2 values. Just needs p[4] and p[5] taken out. Do you want me to spin a secondary patch or do you want to put them together?Acked-by Andrew Hendry <[email protected]>Quoted 59 lines · Dan Rosenberg
Sorry I didn't catch this at the same time as my previous report so it could be included in one patch.
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 <[email protected]>CC stable <[email protected]>---
net/x25/x25_facilities.c | 8 ++++++++1 files changed, 8 insertions(+), 0 deletions(-)net/x25/x25_facilities.c +0
-0@@ -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) {
caseX25_FAC_REVERSE:
if((p[1] &0x81) ==0x81) {net/x25/x25_facilities.c +0
-0@@ -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) {
caseX25_FAC_PACKET_SIZE:
facilities->pacsize_in = p[1];net/x25/x25_facilities.c +0
-0@@ -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]);net/x25/x25_facilities.c +0
-0@@ -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) {
caseX25_FAC_CALLING_AE:
if (p[1] >X25_MAX_DTE_FACIL_LEN|| p[1] <= 1)