[PATCH] prevent buffer overruns when parsing invalid OBEX frames

[PATCH] prevent buffer overruns when parsing invalid OBEX frames

[Iain Hibbert]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 464 bytes --]

Hi,

While I have git installed then, I found a problem recently where hcidump
would dump core when parsing an OBEX packet. It turned out that the OBEX
packet was faulty at my end (socket buffer exhausted), but the patch
attached prevents the parsing code from running off the end of the buffer
and receiving a SIGSEGV.

The patch does not make any complaint about the invalid frame, adding that
would be a little more complex and I'm not sure its necessary?

iain

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2475 bytes --]

From 0a163656b73140265cdb7643ab4596fd3efef62c Mon Sep 17 00:00:00 2001
From: Iain Hibbert <plunky@rya-online.net>
Date: Wed, 17 Feb 2010 18:58:37 +0000
Subject: [PATCH] prevent buffer overruns when parsing invalid OBEX frames

---
 parser/obex.c |   47 +++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/parser/obex.c b/parser/obex.c
index 133f2a2..50b9737 100644
--- a/parser/obex.c
+++ b/parser/obex.c
@@ -200,27 +200,55 @@ static void parse_headers(int level, struct frame *frm)
 		printf("%s (0x%02x)", hi2str(hi), hi);
 		switch (hi & 0xc0) {
 		case 0x00:	/* Unicode */
+			if (frm->len < 2) {
+				printf("\n");
+				return;
+			}
+
 			len = get_u16(frm) - 3;
 			printf(" = Unicode length %d\n", len);
+
+			if (frm->len < len)
+				return;
+
 			raw_ndump(level, frm, len);
 			frm->ptr += len;
 			frm->len -= len;
 			break;
 
 		case 0x40:	/* Byte sequence */
+			if (frm->len < 2) {
+				printf("\n");
+				return;
+			}
+
 			len = get_u16(frm) - 3;
 			printf(" = Sequence length %d\n", len);
+
+			if (frm->len < len)
+				return;
+
 			raw_ndump(level, frm, len);
 			frm->ptr += len;
 			frm->len -= len;
 			break;
 
 		case 0x80:	/* One byte */
+			if (frm->len < 1) {
+				printf("\n");
+				return;
+			}
+
 			hv8 = get_u8(frm);
 			printf(" = %d\n", hv8);
 			break;
 
 		case 0xc0:	/* Four bytes */
+			if (frm->len < 4) {
+				printf("\n");
+				return;
+			}
+
 			hv32 = get_u32(frm);
 			printf(" = %u\n", hv32);
 			break;
@@ -276,6 +304,11 @@ void obex_dump(int level, struct frame *frm)
 
 		switch (opcode & 0x7f) {
 		case 0x00:	/* Connect */
+			if (frm->len < 4) {
+				printf("\n");
+				return;
+			}
+
 			version = get_u8(frm);
 			flags   = get_u8(frm);
 			pktlen  = get_u16(frm);
@@ -284,17 +317,19 @@ void obex_dump(int level, struct frame *frm)
 			break;
 
 		case 0x05:	/* SetPath */
-			if (length > 3) {
-				flags     = get_u8(frm);
-				constants = get_u8(frm);
-				printf(" flags %d constants %d\n",
-							flags, constants);
-			} else
+			if (frm->len < 2) {
 				printf("\n");
+				return;
+			}
+
+			flags     = get_u8(frm);
+			constants = get_u8(frm);
+			printf(" flags %d constants %d\n", flags, constants);
 			break;
 
 		default:
 			printf("\n");
+			break;
 		}
 
 		if ((status & 0x70) && (parser.flags & DUMP_VERBOSE)) {
-- 
1.6.6

Re: [PATCH] prevent buffer overruns when parsing invalid OBEX frames

[Marcel Holtmann]

Hi Iain,

> While I have git installed then, I found a problem recently where hcidump
> would dump core when parsing an OBEX packet. It turned out that the OBEX
> packet was faulty at my end (socket buffer exhausted), but the patch
> attached prevents the parsing code from running off the end of the buffer
> and receiving a SIGSEGV.
> 
> The patch does not make any complaint about the invalid frame, adding that
> would be a little more complex and I'm not sure its necessary?

patch has been applied. Thanks.

Regards

Marcel