* [Bluez-devel] SDP payload processing vulnerability
@ 2008-06-16 20:27 Glenn Durfee
2008-06-16 20:49 ` Glenn Durfee
2008-06-16 20:59 ` Marcel Holtmann
0 siblings, 2 replies; 5+ messages in thread
From: Glenn Durfee @ 2008-06-16 20:27 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 2773 bytes --]
Hi all,
The SDP parsing code blindly trusts string length fields in incoming
SDP packets, exposing reliant applications to over-the-wireless memory
manipulation attacks. An attacker need only send a malformed
response to an SDP query to take advantage of this.
This is most apparent in file bluez-libs-3.30/src/sdp.c, lines 988,
994, 1002 (see below). Also elsewhere in the code where input
pointers are advanced without checking bytes remaining to be parsed.
The root of the problem is that in bluez-libs-3.30/src/sdp.c:1125, the
function sdp_extract_pdu() takes a buffer to parse (in) and a pointer
to a length field (out), but it does not take an incoming length field
(in).
Attached is a patch to fix this issue. Basically I added a
"bytesleft" argument to all of the SDP payload processing routines;
length fields are checked
against the number of remaining bytes to ensure the parser doesn't run
past the end of the packet, or do crazy things like malloc two gigs of
memory. This touches a lot of places, and changes the external API
for SDP payload processing, but I don't see any other way to do this
-- the parser MUST be aware of the incoming packet size in order for
this to be secure.
Glenn
bluez-libs-3.30/src/sdp.c:
972 static sdp_data_t *extract_str(const void *p, int *len)
973 {
974 char *s;
975 int n;
976 sdp_data_t *d = malloc(sizeof(sdp_data_t));
977
978 memset(d, 0, sizeof(sdp_data_t));
979 d->dtd = *(uint8_t *) p;
980 p += sizeof(uint8_t);
981 *len += sizeof(uint8_t);
982
983 switch (d->dtd) {
984 case SDP_TEXT_STR8:
985 case SDP_URL_STR8:
986 n = *(uint8_t *) p; // <-- from the incoming packet
987 p += sizeof(uint8_t);
988 *len += sizeof(uint8_t) + n; // <-- blindly
trusted here, may advance parser past end of packet
989 break;
990 case SDP_TEXT_STR16:
991 case SDP_URL_STR16:
992 n = ntohs(bt_get_unaligned((uint16_t *) p)); //
<-- from the incoming packet
993 p += sizeof(uint16_t);
994 *len += sizeof(uint16_t) + n; // <-- blindly
trusted here, may advance parser past end of packet
995 break;
996 default:
997 SDPERR("Sizeof text string > UINT16_MAX\n");
998 free(d);
999 return 0;
1000 }
1001
1002 s = malloc(n + 1); // <-- really blindly trusted here,
also no NULL checking
1003 memset(s, 0, n + 1);
1004 memcpy(s, p, n);
1005
1006 SDPDBG("Len : %d\n", n);
1007 SDPDBG("Str : %s\n", s);
1008
1009 d->val.str = s;
1010 d->unitSize = n + sizeof(uint8_t); // <-- more blind trust
1011 return d;
1012 }
[-- Attachment #2: bluetooth.patch --]
[-- Type: application/octet-stream, Size: 40388 bytes --]
diff -ur bluetooth.orig/bluez-libs-3.30/include/bluetooth/sdp_lib.h bluetooth/bluez-libs-3.30/include/bluetooth/sdp_lib.h
--- bluetooth.orig/bluez-libs-3.30/include/bluetooth/sdp_lib.h 2008-06-13 13:36:13.913274000 -0700
+++ bluetooth/bluez-libs-3.30/include/bluetooth/sdp_lib.h 2008-06-13 12:06:26.929565000 -0700
@@ -478,7 +478,7 @@
void sdp_uuid32_to_uuid128(uuid_t *uuid128, uuid_t *uuid32);
int sdp_uuid128_to_uuid(uuid_t *uuid);
int sdp_uuid_to_proto(uuid_t *uuid);
-int sdp_uuid_extract(const uint8_t *buffer, uuid_t *uuid, int *scanned);
+int sdp_uuid_extract(const uint8_t *buffer, uuid_t *uuid, int *scanned, int bytesleft);
void sdp_uuid_print(const uuid_t *uuid);
#define MAX_LEN_UUID_STR 37
@@ -584,8 +584,7 @@
return sdp_get_string_attr(rec, SDP_ATTR_ICON_URL, str, len);
}
-sdp_record_t *sdp_extract_pdu(const uint8_t *pdata, int *scanned);
-sdp_data_t *sdp_extract_string(uint8_t *, int *);
+sdp_record_t *sdp_extract_pdu(const uint8_t *pdata, int *scanned, int packet_size);
void sdp_data_print(sdp_data_t *data);
void sdp_print_service_attr(sdp_list_t *alist);
@@ -600,9 +599,9 @@
int sdp_gen_pdu(sdp_buf_t *pdu, sdp_data_t *data);
int sdp_gen_record_pdu(const sdp_record_t *rec, sdp_buf_t *pdu);
-int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *seqlen);
+int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *seqlen, int bytesleft);
-sdp_data_t *sdp_extract_attr(const uint8_t *pdata, int *extractedLength, sdp_record_t *rec);
+sdp_data_t *sdp_extract_attr(const uint8_t *pdata, int *extractedLength, sdp_record_t *rec, int bytesleft);
void sdp_pattern_add_uuid(sdp_record_t *rec, uuid_t *uuid);
void sdp_pattern_add_uuidseq(sdp_record_t *rec, sdp_list_t *seq);
diff -ur bluetooth.orig/bluez-libs-3.30/src/sdp.c bluetooth/bluez-libs-3.30/src/sdp.c
--- bluetooth.orig/bluez-libs-3.30/src/sdp.c 2008-06-13 13:36:13.990281000 -0700
+++ bluetooth/bluez-libs-3.30/src/sdp.c 2008-06-13 12:34:14.999932000 -0700
@@ -904,43 +904,81 @@
free(d);
}
-static sdp_data_t *extract_int(const void *p, int *len)
+static sdp_data_t *extract_int(const void *p, int *len, int bytesleft)
{
sdp_data_t *d = malloc(sizeof(sdp_data_t));
SDPDBG("Extracting integer\n");
memset(d, 0, sizeof(sdp_data_t));
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
+
d->dtd = *(uint8_t *) p;
p += sizeof(uint8_t);
*len += sizeof(uint8_t);
-
+ bytesleft -= sizeof(uint8_t);
+
switch (d->dtd) {
case SDP_DATA_NIL:
break;
case SDP_BOOL:
case SDP_INT8:
case SDP_UINT8:
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
d->val.uint8 = *(uint8_t *) p;
break;
case SDP_INT16:
case SDP_UINT16:
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
d->val.uint16 = ntohs(bt_get_unaligned((uint16_t *) p));
break;
case SDP_INT32:
case SDP_UINT32:
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
d->val.uint32 = ntohl(bt_get_unaligned((uint32_t *) p));
break;
case SDP_INT64:
case SDP_UINT64:
+ if (bytesleft < sizeof(uint64_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint64_t);
+ bytesleft -= sizeof(uint64_t);
d->val.uint64 = ntoh64(bt_get_unaligned((uint64_t *) p));
break;
case SDP_INT128:
case SDP_UINT128:
+ if (bytesleft < sizeof(uint128_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint128_t);
+ bytesleft -= sizeof(uint128_t);
ntoh128((uint128_t *) p, &d->val.uint128);
break;
default:
@@ -950,16 +988,21 @@
return d;
}
-static sdp_data_t *extract_uuid(const uint8_t *p, int *len, sdp_record_t *rec)
+static sdp_data_t *extract_uuid(const uint8_t *p, int *len, sdp_record_t *rec, int bytesleft)
{
sdp_data_t *d = malloc(sizeof(sdp_data_t));
SDPDBG("Extracting UUID");
memset(d, 0, sizeof(sdp_data_t));
- if (sdp_uuid_extract(p, &d->val.uuid, len) < 0) {
+ if (sdp_uuid_extract(p, &d->val.uuid, len, bytesleft) < 0) {
free(d);
return NULL;
}
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
d->dtd = *(uint8_t *) p;
if (rec)
sdp_pattern_add_uuid(rec, &d->val.uuid);
@@ -969,29 +1012,63 @@
/*
* Extract strings from the PDU (could be service description and similar info)
*/
-static sdp_data_t *extract_str(const void *p, int *len)
+static sdp_data_t *extract_str(const void *p, int *len, int bytesleft)
{
char *s;
int n;
sdp_data_t *d = malloc(sizeof(sdp_data_t));
memset(d, 0, sizeof(sdp_data_t));
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
+
d->dtd = *(uint8_t *) p;
p += sizeof(uint8_t);
*len += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
switch (d->dtd) {
case SDP_TEXT_STR8:
case SDP_URL_STR8:
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
n = *(uint8_t *) p;
p += sizeof(uint8_t);
- *len += sizeof(uint8_t) + n;
+ *len += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
+ if (n <= 0 || n > bytesleft) {
+ SDPERR("Invalid length of text string");
+ free(d);
+ return 0;
+ }
+ *len += n;
+ bytesleft -= n;
break;
case SDP_TEXT_STR16:
case SDP_URL_STR16:
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
n = ntohs(bt_get_unaligned((uint16_t *) p));
p += sizeof(uint16_t);
- *len += sizeof(uint16_t) + n;
+ *len += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
+ if (n <= 0 || n > bytesleft) {
+ SDPERR("Invalid length of text string");
+ free(d);
+ return 0;
+ }
+ *len += n;
+ bytesleft -= n;
break;
default:
SDPERR("Sizeof text string > UINT16_MAX\n");
@@ -1000,6 +1077,12 @@
}
s = malloc(n + 1);
+ if (NULL == s) {
+ SDPERR("Cannot allocate memory for string");
+ free(d);
+ return 0;
+ }
+
memset(s, 0, n + 1);
memcpy(s, p, n);
@@ -1011,7 +1094,7 @@
return d;
}
-static sdp_data_t *extract_seq(const void *p, int *len, sdp_record_t *rec)
+static sdp_data_t *extract_seq(const void *p, int *len, sdp_record_t *rec, int bytesleft)
{
int seqlen, n = 0;
sdp_data_t *curr, *prev;
@@ -1019,17 +1102,24 @@
SDPDBG("Extracting SEQ");
memset(d, 0, sizeof(sdp_data_t));
- *len = sdp_extract_seqtype(p, &d->dtd, &seqlen);
+ *len = sdp_extract_seqtype(p, &d->dtd, &seqlen, bytesleft);
SDPDBG("Sequence Type : 0x%x length : 0x%x\n", d->dtd, seqlen);
if (*len == 0)
return d;
+ if (bytesleft < *len) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
+
p += *len;
+ bytesleft -= *len;
curr = prev = NULL;
while (n < seqlen) {
int attrlen = 0;
- curr = sdp_extract_attr(p, &attrlen, rec);
+ curr = sdp_extract_attr(p, &attrlen, rec, bytesleft);
if (curr == NULL)
break;
@@ -1040,6 +1130,7 @@
prev = curr;
p += attrlen;
n += attrlen;
+ bytesleft -= attrlen;
SDPDBG("Extracted: %d SequenceLength: %d", n, seqlen);
}
@@ -1048,10 +1139,16 @@
return d;
}
-sdp_data_t *sdp_extract_attr(const uint8_t *p, int *size, sdp_record_t *rec)
+sdp_data_t *sdp_extract_attr(const uint8_t *p, int *size, sdp_record_t *rec, int bytesleft)
{
sdp_data_t *elem;
int n = 0;
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
+
uint8_t dtd = *(const uint8_t *)p;
SDPDBG("extract_attr: dtd=0x%x", dtd);
@@ -1068,12 +1165,12 @@
case SDP_INT32:
case SDP_INT64:
case SDP_INT128:
- elem = extract_int(p, &n);
+ elem = extract_int(p, &n, bytesleft);
break;
case SDP_UUID16:
case SDP_UUID32:
case SDP_UUID128:
- elem = extract_uuid(p, &n, rec);
+ elem = extract_uuid(p, &n, rec, bytesleft);
break;
case SDP_TEXT_STR8:
case SDP_TEXT_STR16:
@@ -1081,7 +1178,7 @@
case SDP_URL_STR8:
case SDP_URL_STR16:
case SDP_URL_STR32:
- elem = extract_str(p, &n);
+ elem = extract_str(p, &n, bytesleft);
break;
case SDP_SEQ8:
case SDP_SEQ16:
@@ -1089,13 +1186,14 @@
case SDP_ALT8:
case SDP_ALT16:
case SDP_ALT32:
- elem = extract_seq(p, &n, rec);
+ elem = extract_seq(p, &n, rec, bytesleft);
break;
default:
SDPERR("Unknown data descriptor : 0x%x terminating\n", dtd);
return NULL;
}
*size += n;
+ bytesleft -= n;
return elem;
}
@@ -1122,21 +1220,37 @@
}
#endif
-sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int *scanned)
+sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int *scanned, int packet_size)
{
int extracted = 0, seqlen = 0;
uint8_t dtd;
uint16_t attr;
sdp_record_t *rec = sdp_record_alloc();
const uint8_t *p = buf;
+ int bytesleft = packet_size;
- *scanned = sdp_extract_seqtype(buf, &dtd, &seqlen);
+ *scanned = sdp_extract_seqtype(buf, &dtd, &seqlen, bytesleft);
+ if (*scanned == 0) {
+ SDPERR("Sequence length 0 of incoming SDP PDU");
+ return rec;
+ }
p += *scanned;
+ bytesleft -= *scanned;
+ if (bytesleft < seqlen) {
+ SDPERR("Insufficient data in packet to contain sequence");
+ return rec;
+ }
+
rec->attrlist = NULL;
while (extracted < seqlen) {
int n = sizeof(uint8_t), attrlen = 0;
sdp_data_t *data = NULL;
+ if (bytesleft < sizeof(uint8_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ break;
+ }
+
SDPDBG("Extract PDU, sequenceLength: %d localExtractedLength: %d", seqlen, extracted);
dtd = *(uint8_t *) p;
attr = ntohs(bt_get_unaligned((uint16_t *) (p + n)));
@@ -1144,7 +1258,8 @@
SDPDBG("DTD of attrId : %d Attr id : 0x%x \n", dtd, attr);
- data = sdp_extract_attr(p + n, &attrlen, rec);
+ bytesleft -= sizeof(uint8_t)+sizeof(uint16_t);
+ data = sdp_extract_attr(p + n, &attrlen, rec, bytesleft);
SDPDBG("Attr id : 0x%x attrValueLength : %d\n", attr, attrlen);
@@ -1162,6 +1277,7 @@
extracted += n;
p += n;
+ bytesleft -= n;
sdp_attr_replace(rec, attr, data);
SDPDBG("Extract PDU, seqLength: %d localExtractedLength: %d",
seqlen, extracted);
@@ -1266,26 +1382,43 @@
* Extract the sequence type and its length, and return offset into buf
* or 0 on failure.
*/
-int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *size)
+int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *size, int bytesleft)
{
uint8_t dtd = *(uint8_t *) buf;
int scanned = sizeof(uint8_t);
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
buf += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
*dtdp = dtd;
switch (dtd) {
case SDP_SEQ8:
case SDP_ALT8:
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
*size = *(uint8_t *) buf;
scanned += sizeof(uint8_t);
break;
case SDP_SEQ16:
case SDP_ALT16:
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
*size = ntohs(bt_get_unaligned((uint16_t *) buf));
scanned += sizeof(uint16_t);
break;
case SDP_SEQ32:
case SDP_ALT32:
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
*size = ntohl(bt_get_unaligned((uint32_t *) buf));
scanned += sizeof(uint32_t);
break;
@@ -2329,9 +2462,16 @@
return 0;
}
-int sdp_uuid_extract(const uint8_t *p, uuid_t *uuid, int *scanned)
+int sdp_uuid_extract(const uint8_t *p, uuid_t *uuid, int *scanned, int bytesleft)
{
- uint8_t type = *(const uint8_t *) p;
+ uint8_t type;
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
+
+ type = *(const uint8_t *) p;
if (!SDP_IS_UUID(type)) {
SDPERR("Unknown data type : %d expecting a svc UUID\n", type);
@@ -2339,17 +2479,34 @@
}
p += sizeof(uint8_t);
*scanned += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
+
if (type == SDP_UUID16) {
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
sdp_uuid16_create(uuid, ntohs(bt_get_unaligned((uint16_t *) p)));
*scanned += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
p += sizeof(uint16_t);
} else if (type == SDP_UUID32) {
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
sdp_uuid32_create(uuid, ntohl(bt_get_unaligned((uint32_t *) p)));
*scanned += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
p += sizeof(uint32_t);
} else {
+ if (bytesleft < sizeof(uint128_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
sdp_uuid128_create(uuid, p);
*scanned += sizeof(uint128_t);
+ bytesleft -= sizeof(uint128_t);
p += sizeof(uint128_t);
}
return 0;
@@ -2447,6 +2604,7 @@
uint32_t reqsize, rspsize;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
int status;
+ int bytesleft;
SDPDBG("");
@@ -2483,9 +2641,19 @@
status = sdp_send_req_w4_rsp(session, req, rsp, reqsize, &rspsize);
if (status < 0)
goto end;
+
+ bytesleft = rspsize;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+
rsphdr = (sdp_pdu_hdr_t *) rsp;
p = rsp + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
/* Invalid service record */
@@ -2495,7 +2663,13 @@
errno = EPROTO;
status = -1;
} else {
- if (handle)
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+ if (handle)
*handle = ntohl(bt_get_unaligned((uint32_t *) p));
}
@@ -2556,6 +2730,7 @@
uint32_t reqsize = 0, rspsize = 0;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
int status;
+ int bytesleft;
SDPDBG("");
@@ -2590,9 +2765,19 @@
if (status < 0)
goto end;
- rsphdr = (sdp_pdu_hdr_t *) rspbuf;
+ bytesleft = rspsize;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+
+ rsphdr = (sdp_pdu_hdr_t *) rspbuf;
p = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
status = bt_get_unaligned((uint16_t *) p);
+ bytesleft -= sizeof(uint16_t);
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
/* For this case the status always is invalid record handle */
@@ -2644,6 +2829,7 @@
uint32_t handle;
sdp_buf_t pdu;
int status;
+ int bytesleft;
SDPDBG("");
@@ -2688,12 +2874,23 @@
status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
if (status < 0)
goto end;
+
+ bytesleft = rspsize;
SDPDBG("Send req status : %d\n", status);
- rsphdr = (sdp_pdu_hdr_t *) rspbuf;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+
+ rsphdr = (sdp_pdu_hdr_t *) rspbuf;
p = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
status = bt_get_unaligned((uint16_t *) p);
+ bytesleft -= sizeof(uint16_t);
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
/* The status can be invalid sintax or invalid record handle */
@@ -2764,18 +2961,23 @@
* handles are not in "data element sequence" form, but just like
* an array of service handles
*/
-static void extract_record_handle_seq(uint8_t *pdu, sdp_list_t **seq, int count, int *scanned)
+static void extract_record_handle_seq(uint8_t *pdu, sdp_list_t **seq, int count, int *scanned, int *bytesleft)
{
sdp_list_t *pSeq = *seq;
uint8_t *pdata = pdu;
int n;
for (n = 0; n < count; n++) {
+ if (*bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ break;
+ }
uint32_t *pSvcRec = malloc(sizeof(uint32_t));
*pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
pSeq = sdp_list_append(pSeq, pSvcRec);
pdata += sizeof(uint32_t);
*scanned += sizeof(uint32_t);
+ *bytesleft -= sizeof(uint32_t);
}
*seq = pSeq;
}
@@ -2843,12 +3045,17 @@
unsigned char data[16];
} __attribute__ ((packed)) sdp_cstate_t;
-static int copy_cstate(uint8_t *pdata, const sdp_cstate_t *cstate)
+static int copy_cstate(uint8_t *pdata, int pdata_len, const sdp_cstate_t *cstate)
{
if (cstate) {
- *pdata++ = cstate->length;
- memcpy(pdata, cstate->data, cstate->length);
- return cstate->length + 1;
+ uint8_t len = cstate->length;
+ if ((int)len > pdata_len) {
+ SDPERR("Continuation state size exceeds internal buffer");
+ } else {
+ *pdata++ = len;
+ memcpy(pdata, cstate->data, len);
+ return len + 1;
+ }
}
*pdata = 0;
return 1;
@@ -2892,9 +3099,11 @@
int seqlen = 0;
int scanned, total_rec_count, rec_count;
uint8_t *pdata, *_pdata;
+ int _pdata_len;
uint8_t *reqbuf, *rspbuf;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
sdp_cstate_t *cstate = NULL;
+ int bytesleft;
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
@@ -2924,11 +3133,12 @@
_reqsize = reqsize;
_pdata = pdata;
+ _pdata_len = SDP_REQ_BUFFER_SIZE - reqsize;
*rsp = NULL;
do {
// Add continuation state or NULL (first time)
- reqsize = _reqsize + copy_cstate(_pdata, cstate);
+ reqsize = _reqsize + copy_cstate(_pdata, _pdata_len, cstate);
// Set the request header's param length
reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
@@ -2940,8 +3150,16 @@
*/
status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
if (status < 0)
- goto end;
-
+ goto end;
+
+ bytesleft = rspsize;
+
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
rsplen = 0;
rsphdr = (sdp_pdu_hdr_t *) rspbuf;
rsplen = ntohs(rsphdr->plen);
@@ -2953,14 +3171,22 @@
}
scanned = 0;
pdata = rspbuf + sizeof(sdp_pdu_hdr_t);
-
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
+ if (bytesleft < sizeof(uint16_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
// net service record match count
total_rec_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
pdata += sizeof(uint16_t);
scanned += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
rec_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
pdata += sizeof(uint16_t);
scanned += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
SDPDBG("Total svc count: %d\n", total_rec_count);
SDPDBG("Current svc count: %d\n", rec_count);
@@ -2970,12 +3196,19 @@
status = -1;
goto end;
}
- extract_record_handle_seq(pdata, rsp, rec_count, &scanned);
+ extract_record_handle_seq(pdata, rsp, rec_count, &scanned, bytesleft);
SDPDBG("BytesScanned : %d\n", scanned);
+ bytesleft -= scanned;
if (rsplen > scanned) {
uint8_t cstate_len;
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet: continuation state data missing");
+ status = -1;
+ goto end;
+ }
+
pdata = rspbuf + sizeof(sdp_pdu_hdr_t) + scanned;
cstate_len = *(uint8_t *) pdata;
if (cstate_len > 0) {
@@ -3036,12 +3269,14 @@
int attr_list_len = 0;
int seqlen = 0;
uint8_t *pdata, *_pdata;
+ int _pdata_len;
uint8_t *reqbuf, *rspbuf;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
sdp_cstate_t *cstate = NULL;
uint8_t cstate_len = 0;
sdp_buf_t rsp_concat_buf;
sdp_record_t *rec = 0;
+ int bytesleft;
if (reqtype != SDP_ATTR_REQ_INDIVIDUAL && reqtype != SDP_ATTR_REQ_RANGE) {
errno = EINVAL;
@@ -3087,10 +3322,11 @@
// save before Continuation State
_pdata = pdata;
_reqsize = reqsize;
+ _pdata_len = SDP_REQ_BUFFER_SIZE - reqsize;
do {
// add NULL continuation state
- reqsize = _reqsize + copy_cstate(_pdata, cstate);
+ reqsize = _reqsize + copy_cstate(_pdata, _pdata_len, cstate);
// set the request header's param length
reqhdr->tid = htons(sdp_gen_tid(session));
@@ -3099,6 +3335,14 @@
status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
if (status < 0)
goto end;
+ bytesleft = rspsize;
+
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
rsp_count = 0;
rsphdr = (sdp_pdu_hdr_t *) rspbuf;
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
@@ -3107,12 +3351,25 @@
goto end;
}
pdata = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
+
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
attr_list_len += rsp_count;
pdata += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
// if continuation state set need to re-issue request before parsing
- cstate_len = *(uint8_t *) (pdata + rsp_count);
+ if (bytesleft < rsp_count + sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet: continuation state data missing");
+ status = -1;
+ goto end;
+ }
+ cstate_len = *(uint8_t *) (pdata + rsp_count);
SDPDBG("Response id : %d\n", rsphdr->pdu_id);
SDPDBG("Attrlist byte count : %d\n", rsp_count);
@@ -3138,9 +3395,11 @@
if (attr_list_len > 0) {
int scanned = 0;
- if (rsp_concat_buf.data_size != 0)
+ if (rsp_concat_buf.data_size != 0) {
pdata = rsp_concat_buf.data;
- rec = sdp_extract_pdu(pdata, &scanned);
+ bytesleft = rsp_concat_buf.data_size;
+ }
+ rec = sdp_extract_pdu(pdata, &scanned, bytesleft);
if (!rec)
status = -1;
@@ -3269,6 +3528,7 @@
sdp_pdu_hdr_t *reqhdr;
uint8_t *pdata;
int cstate_len, seqlen = 0;
+ int pdata_len;
if (!session || !session->priv)
return -1;
@@ -3296,6 +3556,7 @@
// generate PDU
pdata = t->reqbuf + sizeof(sdp_pdu_hdr_t);
t->reqsize = sizeof(sdp_pdu_hdr_t);
+ pdata_len = SDP_REQ_BUFFER_SIZE - t->reqsize;
// add service class IDs for search
seqlen = gen_searchseq_pdu(pdata, search);
@@ -3305,13 +3566,15 @@
// now set the length and increment the pointer
t->reqsize += seqlen;
pdata += seqlen;
+ pdata_len -= seqlen;
bt_put_unaligned(htons(max_rec_num), (uint16_t *) pdata);
t->reqsize += sizeof(uint16_t);
pdata += sizeof(uint16_t);
+ pdata_len -= sizeof(uint16_t);
// set the request header's param length
- cstate_len = copy_cstate(pdata, NULL);
+ cstate_len = copy_cstate(pdata, pdata_len, NULL);
reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
@@ -3373,6 +3636,7 @@
sdp_pdu_hdr_t *reqhdr;
uint8_t *pdata;
int cstate_len, seqlen = 0;
+ int pdata_len;
if (!session || !session->priv)
return -1;
@@ -3400,16 +3664,19 @@
// generate PDU
pdata = t->reqbuf + sizeof(sdp_pdu_hdr_t);
t->reqsize = sizeof(sdp_pdu_hdr_t);
+ pdata_len = SDP_REQ_BUFFER_SIZE - t->reqsize;
// add the service record handle
bt_put_unaligned(htonl(handle), (uint32_t *) pdata);
t->reqsize += sizeof(uint32_t);
pdata += sizeof(uint32_t);
+ pdata_len -= sizeof(uint32_t);
// specify the response limit
bt_put_unaligned(htons(65535), (uint16_t *) pdata);
t->reqsize += sizeof(uint16_t);
pdata += sizeof(uint16_t);
+ pdata_len -= sizeof(uint16_t);
// get attr seq PDU form
seqlen = gen_attridseq_pdu(pdata, attrid_list,
@@ -3422,10 +3689,11 @@
// now set the length and increment the pointer
t->reqsize += seqlen;
pdata += seqlen;
+ pdata_len -= seqlen;
SDPDBG("Attr list length : %d\n", seqlen);
// set the request header's param length
- cstate_len = copy_cstate(pdata, NULL);
+ cstate_len = copy_cstate(pdata, pdata_len, NULL);
reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
@@ -3488,6 +3756,7 @@
sdp_pdu_hdr_t *reqhdr;
uint8_t *pdata;
int cstate_len, seqlen = 0;
+ int pdata_len;
if (!session || !session->priv)
return -1;
@@ -3515,6 +3784,7 @@
// generate PDU
pdata = t->reqbuf + sizeof(sdp_pdu_hdr_t);
t->reqsize = sizeof(sdp_pdu_hdr_t);
+ pdata_len = SDP_REQ_BUFFER_SIZE - t->reqsize;
// add service class IDs for search
seqlen = gen_searchseq_pdu(pdata, search);
@@ -3524,10 +3794,12 @@
// now set the length and increment the pointer
t->reqsize += seqlen;
pdata += seqlen;
+ pdata_len -= seqlen;
bt_put_unaligned(htons(SDP_MAX_ATTR_LEN), (uint16_t *) pdata);
t->reqsize += sizeof(uint16_t);
pdata += sizeof(uint16_t);
+ pdata_len -= sizeof(uint16_t);
SDPDBG("Max attr byte count : %d\n", SDP_MAX_ATTR_LEN);
@@ -3540,11 +3812,12 @@
}
pdata += seqlen;
+ pdata_len -= seqlen;
SDPDBG("Attr list length : %d\n", seqlen);
t->reqsize += seqlen;
// set the request header's param length
- cstate_len = copy_cstate(pdata, NULL);
+ cstate_len = copy_cstate(pdata, pdata_len, NULL);
reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
@@ -3757,7 +4030,7 @@
reqhdr->tid = htons(sdp_gen_tid(session));
// add continuation state
- cstate_len = copy_cstate(t->reqbuf + t->reqsize, pcstate);
+ cstate_len = copy_cstate(t->reqbuf + t->reqsize, SDP_REQ_BUFFER_SIZE - t->reqsize, pcstate);
reqsize = t->reqsize + cstate_len;
@@ -3838,12 +4111,14 @@
int seqlen = 0, attr_list_len = 0;
int rsp_count = 0, cstate_len = 0;
uint8_t *pdata, *_pdata;
+ int _pdata_len;
uint8_t *reqbuf, *rspbuf;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
uint8_t dataType;
sdp_list_t *rec_list = NULL;
sdp_buf_t rsp_concat_buf;
sdp_cstate_t *cstate = NULL;
+ int bytesleft;
if (reqtype != SDP_ATTR_REQ_INDIVIDUAL && reqtype != SDP_ATTR_REQ_RANGE) {
errno = EINVAL;
@@ -3895,12 +4170,13 @@
// save before Continuation State
_pdata = pdata;
_reqsize = reqsize;
+ _pdata_len = SDP_REQ_BUFFER_SIZE - reqsize;
do {
reqhdr->tid = htons(sdp_gen_tid(session));
// add continuation state (can be null)
- reqsize = _reqsize + copy_cstate(_pdata, cstate);
+ reqsize = _reqsize + copy_cstate(_pdata, _pdata_len, cstate);
// set the request header's param length
reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
@@ -3910,6 +4186,13 @@
SDPDBG("Status : 0x%x\n", rsphdr->pdu_id);
goto end;
}
+
+ bytesleft = rspsize;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
status = -1;
@@ -3917,9 +4200,25 @@
}
pdata = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
+
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
attr_list_len += rsp_count;
pdata += sizeof(uint16_t); // pdata points to attribute list
+ bytesleft -= sizeof(uint16_t);
+
+ if (bytesleft < rsp_count) {
+ SDPERR("Unexpected end of packet: continuation state data missing");
+ status = -1;
+ goto end;
+ }
+
cstate_len = *(uint8_t *) (pdata + rsp_count);
SDPDBG("Attrlist byte count : %d\n", attr_list_len);
@@ -3946,24 +4245,27 @@
if (attr_list_len > 0) {
int scanned = 0;
- if (rsp_concat_buf.data_size != 0)
+ if (rsp_concat_buf.data_size != 0) {
pdata = rsp_concat_buf.data;
+ bytesleft = rsp_concat_buf.data_size;
+ }
/*
* Response is a sequence of sequence(s) for one or
* more data element sequence(s) representing services
* for which attributes are returned
*/
- scanned = sdp_extract_seqtype(pdata, &dataType, &seqlen);
+ scanned = sdp_extract_seqtype(pdata, &dataType, &seqlen, bytesleft);
SDPDBG("Bytes scanned : %d\n", scanned);
SDPDBG("Seq length : %d\n", seqlen);
if (scanned && seqlen) {
pdata += scanned;
+ bytesleft -= scanned;
do {
int recsize = 0;
- sdp_record_t *rec = sdp_extract_pdu(pdata, &recsize);
+ sdp_record_t *rec = sdp_extract_pdu(pdata, &recsize, bytesleft);
if (rec == NULL) {
SDPERR("SVC REC is null\n");
status = -1;
@@ -3975,6 +4277,7 @@
}
scanned += recsize;
pdata += recsize;
+ bytesleft -= recsize;
SDPDBG("Loc seq length : %d\n", recsize);
SDPDBG("Svc Rec Handle : 0x%x\n", rec->handle);
diff -ur bluetooth.orig/bluez-utils-3.30/audio/headset.c bluetooth/bluez-utils-3.30/audio/headset.c
--- bluetooth.orig/bluez-utils-3.30/audio/headset.c 2008-06-13 13:36:14.201284000 -0700
+++ bluetooth/bluez-utils-3.30/audio/headset.c 2008-06-13 12:37:15.957158000 -0700
@@ -848,7 +848,7 @@
goto failed_not_supported;
}
- record = sdp_extract_pdu(array, &record_len);
+ record = sdp_extract_pdu(array, &record_len, array_len);
if (!record) {
error("Unable to extract service record from reply");
goto failed_not_supported;
diff -ur bluetooth.orig/bluez-utils-3.30/audio/manager.c bluetooth/bluez-utils-3.30/audio/manager.c
--- bluetooth.orig/bluez-utils-3.30/audio/manager.c 2008-06-13 13:36:14.204281000 -0700
+++ bluetooth/bluez-utils-3.30/audio/manager.c 2008-06-13 12:37:44.757153000 -0700
@@ -419,7 +419,7 @@
goto failed;
}
- record = sdp_extract_pdu(array, &record_len);
+ record = sdp_extract_pdu(array, &record_len, array_len);
if (!record) {
error("Unable to extract service record from reply");
goto done;
diff -ur bluetooth.orig/bluez-utils-3.30/common/glib-helper.c bluetooth/bluez-utils-3.30/common/glib-helper.c
--- bluetooth.orig/bluez-utils-3.30/common/glib-helper.c 2008-06-13 13:36:14.300310000 -0700
+++ bluetooth/bluez-utils-3.30/common/glib-helper.c 2008-06-13 12:25:08.683864000 -0700
@@ -62,23 +62,25 @@
int scanned, seqlen = 0;
uint8_t dataType;
int err = 0;
+ int bytesleft = size;
if (status || type != SDP_SVC_SEARCH_ATTR_RSP) {
err = -EPROTO;
goto done;
}
- scanned = sdp_extract_seqtype(rsp, &dataType, &seqlen);
+ scanned = sdp_extract_seqtype(rsp, &dataType, &seqlen, bytesleft);
if (!scanned || !seqlen)
goto done;
rsp += scanned;
+ bytesleft -= scanned;
do {
sdp_record_t *rec;
int recsize;
recsize = 0;
- rec = sdp_extract_pdu(rsp, &recsize);
+ rec = sdp_extract_pdu(rsp, &recsize, bytesleft);
if (!rec)
break;
@@ -89,6 +91,7 @@
scanned += recsize;
rsp += recsize;
+ bytesleft -= recsize;
recs = sdp_list_append(recs, rec);
} while (scanned < size);
diff -ur bluetooth.orig/bluez-utils-3.30/hcid/dbus-database.c bluetooth/bluez-utils-3.30/hcid/dbus-database.c
--- bluetooth.orig/bluez-utils-3.30/hcid/dbus-database.c 2008-06-13 13:36:14.708148000 -0700
+++ bluetooth/bluez-utils-3.30/hcid/dbus-database.c 2008-06-13 12:27:57.935715000 -0700
@@ -107,7 +107,7 @@
if (len <= 0)
return error_invalid_arguments(conn, msg, NULL);
- sdp_record = sdp_extract_pdu(record, &scanned);
+ sdp_record = sdp_extract_pdu(record, &scanned, len);
if (!sdp_record) {
error("Parsing of service record failed");
return error_failed_errno(conn, msg, EIO);
@@ -253,7 +253,7 @@
if (!user_record)
return error_not_available(conn, msg);
- sdp_record = sdp_extract_pdu(bin_record, &scanned);
+ sdp_record = sdp_extract_pdu(bin_record, &scanned, size);
if (!sdp_record) {
error("Parsing of service record failed");
return error_invalid_arguments(conn, msg, NULL);
diff -ur bluetooth.orig/bluez-utils-3.30/hcid/dbus-sdp.c bluetooth/bluez-utils-3.30/hcid/dbus-sdp.c
--- bluetooth.orig/bluez-utils-3.30/hcid/dbus-sdp.c 2008-06-13 13:36:14.711144000 -0700
+++ bluetooth/bluez-utils-3.30/hcid/dbus-sdp.c 2008-06-13 13:25:14.830806000 -0700
@@ -494,7 +494,7 @@
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
DBUS_TYPE_BYTE_AS_STRING, &array_iter);
- rec = sdp_extract_pdu(rsp, &scanned);
+ rec = sdp_extract_pdu(rsp, &scanned, size);
if (rec == NULL || size != scanned) {
error("Invalid service record!");
goto done;
@@ -563,7 +563,7 @@
reply = dbus_message_new_method_return(ctxt->rq);
- rec = sdp_extract_pdu(rsp, &scanned);
+ rec = sdp_extract_pdu(rsp, &scanned, size);
if (rec == NULL || size != scanned) {
error("Invalid service record!");
goto done;
@@ -731,6 +731,7 @@
GSList *l = NULL;
int scanned, extracted = 0, len = 0, recsize = 0;
uint8_t dtd = 0;
+ int bytesleft = (int)size;
if (!ctxt)
return;
@@ -766,16 +767,18 @@
DBUS_TYPE_STRING, &dst,
DBUS_TYPE_INVALID);
- scanned = sdp_extract_seqtype(rsp, &dtd, &len);
+ scanned = sdp_extract_seqtype(rsp, &dtd, &len, bytesleft);
rsp += scanned;
+ bytesleft -= scanned;
for (; extracted < len; rsp += recsize, extracted += recsize) {
sdp_record_t *rec;
sdp_data_t *d;
recsize = 0;
- rec = sdp_extract_pdu(rsp, &recsize);
+ rec = sdp_extract_pdu(rsp, &recsize, bytesleft);
if (!rec)
break;
+ bytesleft -= recsize;
sdp_store_record(src, dst, rec->handle, rsp, recsize);
diff -ur bluetooth.orig/bluez-utils-3.30/input/manager.c bluetooth/bluez-utils-3.30/input/manager.c
--- bluetooth.orig/bluez-utils-3.30/input/manager.c 2008-06-13 13:36:15.400156000 -0700
+++ bluetooth/bluez-utils-3.30/input/manager.c 2008-06-13 12:39:37.109807000 -0700
@@ -536,7 +536,7 @@
goto fail;
}
- pr->hid_rec = sdp_extract_pdu(rec_bin, &scanned);
+ pr->hid_rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (!pr->hid_rec) {
error_not_supported(pr->conn, pr->msg);
goto fail;
@@ -673,7 +673,7 @@
goto fail;
}
- pr->pnp_rec = sdp_extract_pdu(rec_bin, &scanned);
+ pr->pnp_rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (get_handles(pr, hid_uuid, hid_handle_reply) < 0) {
error_not_supported(pr->conn, pr->msg);
error("HID service search request failed");
@@ -793,7 +793,7 @@
goto fail;
}
- rec = sdp_extract_pdu(rec_bin, &scanned);
+ rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (!rec) {
error_not_supported(pr->conn, pr->msg);
goto fail;
diff -ur bluetooth.orig/bluez-utils-3.30/network/manager.c bluetooth/bluez-utils-3.30/network/manager.c
--- bluetooth.orig/bluez-utils-3.30/network/manager.c 2008-06-13 13:36:15.503140000 -0700
+++ bluetooth/bluez-utils-3.30/network/manager.c 2008-06-13 12:40:13.287871000 -0700
@@ -253,7 +253,7 @@
goto fail;
}
- rec = sdp_extract_pdu(rec_bin, &scanned);
+ rec = sdp_extract_pdu(rec_bin, &scanned, len);
/* Concat remote name and service name */
memset(name, 0, MAX_NAME_SIZE);
diff -ur bluetooth.orig/bluez-utils-3.30/sdpd/request.c bluetooth/bluez-utils-3.30/sdpd/request.c
--- bluetooth.orig/bluez-utils-3.30/sdpd/request.c 2008-06-13 13:36:15.720000000 -0700
+++ bluetooth/bluez-utils-3.30/sdpd/request.c 2008-06-13 13:30:53.543764000 -0700
@@ -67,8 +67,9 @@
uint8_t dataType;
int status = 0;
const uint8_t *p;
+ int bytesleft = len;
- scanned = sdp_extract_seqtype(buf, &seqType, &data_size);
+ scanned = sdp_extract_seqtype(buf, &seqType, &data_size, bytesleft);
debug("Seq type : %d", seqType);
if (!scanned || (seqType != SDP_SEQ8 && seqType != SDP_SEQ16)) {
@@ -76,12 +77,18 @@
return -1;
}
p = buf + scanned;
+ bytesleft -= scanned;
debug("Data size : %d", data_size);
for (;;) {
char *pElem = NULL;
int localSeqLength = 0;
+
+ if (bytesleft < sizeof(uint8_t)) {
+ debug("->Unexpected end of buffer");
+ return -1;
+ }
dataType = *(uint8_t *)p;
debug("Data type: 0x%02x", dataType);
@@ -100,6 +107,12 @@
case SDP_UINT16:
p += sizeof(uint8_t);
seqlen += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
+ if (bytesleft < sizeof(uint16_t)) {
+ debug("->Unexpected end of buffer");
+ return -1;
+ }
+
pElem = malloc(sizeof(uint16_t));
bt_put_unaligned(ntohs(bt_get_unaligned((uint16_t *)p)), (uint16_t *)pElem);
p += sizeof(uint16_t);
@@ -108,7 +121,13 @@
case SDP_UINT32:
p += sizeof(uint8_t);
seqlen += sizeof(uint8_t);
- pElem = malloc(sizeof(uint32_t));
+ bytesleft -= sizeof(uint8_t);
+ if (bytesleft < sizeof(uint32_t)) {
+ debug("->Unexpected end of buffer");
+ return -1;
+ }
+
+ pElem = malloc(sizeof(uint32_t));
bt_put_unaligned(ntohl(bt_get_unaligned((uint32_t *)p)), (uint32_t *)pElem);
p += sizeof(uint32_t);
seqlen += sizeof(uint32_t);
@@ -117,10 +136,11 @@
case SDP_UUID32:
case SDP_UUID128:
pElem = malloc(sizeof(uuid_t));
- status = sdp_uuid_extract(p, (uuid_t *)pElem, &localSeqLength);
+ status = sdp_uuid_extract(p, (uuid_t *)pElem, &localSeqLength, bytesleft);
if (status == 0) {
seqlen += localSeqLength;
p += localSeqLength;
+ bytesleft -= localSeqLength;
}
break;
default:
diff -ur bluetooth.orig/bluez-utils-3.30/sdpd/service.c bluetooth/bluez-utils-3.30/sdpd/service.c
--- bluetooth.orig/bluez-utils-3.30/sdpd/service.c 2008-06-13 13:36:15.716011000 -0700
+++ bluetooth/bluez-utils-3.30/sdpd/service.c 2008-06-13 12:53:10.911126000 -0700
@@ -412,7 +412,7 @@
}
// FIXME: refactor for server-side
-static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t handleExpected, int *scanned)
+static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t handleExpected, int *scanned, int bytesleft)
{
int extractStatus = -1, localExtractedLength = 0;
uint8_t dtd;
@@ -422,8 +422,9 @@
sdp_data_t *pAttr = NULL;
uint32_t handle = 0xffffffff;
- *scanned = sdp_extract_seqtype(p, &dtd, &seqlen);
+ *scanned = sdp_extract_seqtype(p, &dtd, &seqlen, bytesleft);
p += *scanned;
+ bytesleft -= *scanned;
lookAheadAttrId = ntohs(bt_get_unaligned((uint16_t *) (p + sizeof(uint8_t))));
debug("Look ahead attr id : %d", lookAheadAttrId);
@@ -464,7 +465,7 @@
debug("DTD of attrId : %d Attr id : 0x%x", dtd, attrId);
- pAttr = sdp_extract_attr(p + attrSize, &attrValueLength, rec);
+ pAttr = sdp_extract_attr(p + attrSize, &attrValueLength, rec, bytesleft - attrSize);
debug("Attr id : 0x%x attrValueLength : %d", attrId, attrValueLength);
@@ -475,6 +476,7 @@
}
localExtractedLength += attrSize;
p += attrSize;
+ bytesleft -= attrSize;
sdp_attr_replace(rec, attrId, pAttr);
extractStatus = 0;
debug("Extract PDU, seqLength: %d localExtractedLength: %d",
@@ -500,15 +502,17 @@
sdp_data_t *handle;
uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
sdp_record_t *rec;
+ int bytesleft = req->len - sizeof(sdp_pdu_hdr_t);
req->flags = *p++;
if (req->flags & SDP_DEVICE_RECORD) {
bacpy(&req->device, (bdaddr_t *) p);
p += sizeof(bdaddr_t);
+ bytesleft -= sizeof(bdaddr_t);
}
// save image of PDU: we need it when clients request this attribute
- rec = extract_pdu_server(&req->device, p, 0xffffffff, &scanned);
+ rec = extract_pdu_server(&req->device, p, 0xffffffff, &scanned, bytesleft);
if (!rec)
goto invalid;
@@ -578,18 +582,20 @@
sdp_record_t *orec;
int status = 0, scanned = 0;
uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
+ int bytesleft = req->len - sizeof(sdp_pdu_hdr_t);
uint32_t handle = ntohl(bt_get_unaligned((uint32_t *) p));
debug("Svc Rec Handle: 0x%x", handle);
p += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
orec = sdp_record_find(handle);
debug("SvcRecOld: %p", orec);
if (orec) {
- sdp_record_t *nrec = extract_pdu_server(BDADDR_ANY, p, handle, &scanned);
+ sdp_record_t *nrec = extract_pdu_server(BDADDR_ANY, p, handle, &scanned, bytesleft);
if (nrec && handle == nrec->handle) {
update_db_timestamp();
update_svclass_list();
diff -ur bluetooth.orig/bluez-utils-3.30/serial/manager.c bluetooth/bluez-utils-3.30/serial/manager.c
--- bluetooth.orig/bluez-utils-3.30/serial/manager.c 2008-06-13 13:36:15.779001000 -0700
+++ bluetooth/bluez-utils-3.30/serial/manager.c 2008-06-13 12:40:48.550090000 -0700
@@ -520,7 +520,7 @@
goto fail;
}
- rec = sdp_extract_pdu(rec_bin, &scanned);
+ rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (!rec) {
error("Can't extract SDP record.");
error_not_supported(pc->conn, pc->msg);
[-- Attachment #3: Type: text/plain, Size: 247 bytes --]
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
[-- Attachment #4: Type: text/plain, Size: 164 bytes --]
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bluez-devel] SDP payload processing vulnerability
2008-06-16 20:27 [Bluez-devel] SDP payload processing vulnerability Glenn Durfee
@ 2008-06-16 20:49 ` Glenn Durfee
2008-06-16 20:59 ` Marcel Holtmann
1 sibling, 0 replies; 5+ messages in thread
From: Glenn Durfee @ 2008-06-16 20:49 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]
Oops, sent the wrong version, meant to send this one instead.
On Mon, Jun 16, 2008 at 1:27 PM, Glenn Durfee <gdurfee@google.com> wrote:
> Hi all,
>
> The SDP parsing code blindly trusts string length fields in incoming
> SDP packets, exposing reliant applications to over-the-wireless memory
> manipulation attacks. An attacker need only send a malformed
> response to an SDP query to take advantage of this.
>
> This is most apparent in file bluez-libs-3.30/src/sdp.c, lines 988,
> 994, 1002 (see below). Also elsewhere in the code where input
> pointers are advanced without checking bytes remaining to be parsed.
> The root of the problem is that in bluez-libs-3.30/src/sdp.c:1125, the
> function sdp_extract_pdu() takes a buffer to parse (in) and a pointer
> to a length field (out), but it does not take an incoming length field
> (in).
>
> Attached is a patch to fix this issue. Basically I added a
> "bytesleft" argument to all of the SDP payload processing routines;
> length fields are checked
> against the number of remaining bytes to ensure the parser doesn't run
> past the end of the packet, or do crazy things like malloc two gigs of
> memory. This touches a lot of places, and changes the external API
> for SDP payload processing, but I don't see any other way to do this
> -- the parser MUST be aware of the incoming packet size in order for
> this to be secure.
>
> Glenn
>
> bluez-libs-3.30/src/sdp.c:
>
> 972 static sdp_data_t *extract_str(const void *p, int *len)
> 973 {
> 974 char *s;
> 975 int n;
> 976 sdp_data_t *d = malloc(sizeof(sdp_data_t));
> 977
> 978 memset(d, 0, sizeof(sdp_data_t));
> 979 d->dtd = *(uint8_t *) p;
> 980 p += sizeof(uint8_t);
> 981 *len += sizeof(uint8_t);
> 982
> 983 switch (d->dtd) {
> 984 case SDP_TEXT_STR8:
> 985 case SDP_URL_STR8:
> 986 n = *(uint8_t *) p; // <-- from the incoming packet
> 987 p += sizeof(uint8_t);
> 988 *len += sizeof(uint8_t) + n; // <-- blindly
> trusted here, may advance parser past end of packet
> 989 break;
> 990 case SDP_TEXT_STR16:
> 991 case SDP_URL_STR16:
> 992 n = ntohs(bt_get_unaligned((uint16_t *) p)); //
> <-- from the incoming packet
> 993 p += sizeof(uint16_t);
> 994 *len += sizeof(uint16_t) + n; // <-- blindly
> trusted here, may advance parser past end of packet
> 995 break;
> 996 default:
> 997 SDPERR("Sizeof text string > UINT16_MAX\n");
> 998 free(d);
> 999 return 0;
> 1000 }
> 1001
> 1002 s = malloc(n + 1); // <-- really blindly trusted here,
> also no NULL checking
> 1003 memset(s, 0, n + 1);
> 1004 memcpy(s, p, n);
> 1005
> 1006 SDPDBG("Len : %d\n", n);
> 1007 SDPDBG("Str : %s\n", s);
> 1008
> 1009 d->val.str = s;
> 1010 d->unitSize = n + sizeof(uint8_t); // <-- more blind trust
> 1011 return d;
> 1012 }
>
[-- Attachment #2: bluetooth.patch.2 --]
[-- Type: application/octet-stream, Size: 40385 bytes --]
diff -ur bluetooth.orig/bluez-libs-3.30/include/bluetooth/sdp_lib.h bluetooth/bluez-libs-3.30/include/bluetooth/sdp_lib.h
--- bluetooth.orig/bluez-libs-3.30/include/bluetooth/sdp_lib.h 2008-06-13 13:36:13.913274000 -0700
+++ bluetooth/bluez-libs-3.30/include/bluetooth/sdp_lib.h 2008-06-13 12:06:26.929565000 -0700
@@ -478,7 +478,7 @@
void sdp_uuid32_to_uuid128(uuid_t *uuid128, uuid_t *uuid32);
int sdp_uuid128_to_uuid(uuid_t *uuid);
int sdp_uuid_to_proto(uuid_t *uuid);
-int sdp_uuid_extract(const uint8_t *buffer, uuid_t *uuid, int *scanned);
+int sdp_uuid_extract(const uint8_t *buffer, uuid_t *uuid, int *scanned, int bytesleft);
void sdp_uuid_print(const uuid_t *uuid);
#define MAX_LEN_UUID_STR 37
@@ -584,8 +584,7 @@
return sdp_get_string_attr(rec, SDP_ATTR_ICON_URL, str, len);
}
-sdp_record_t *sdp_extract_pdu(const uint8_t *pdata, int *scanned);
-sdp_data_t *sdp_extract_string(uint8_t *, int *);
+sdp_record_t *sdp_extract_pdu(const uint8_t *pdata, int *scanned, int packet_size);
void sdp_data_print(sdp_data_t *data);
void sdp_print_service_attr(sdp_list_t *alist);
@@ -600,9 +599,9 @@
int sdp_gen_pdu(sdp_buf_t *pdu, sdp_data_t *data);
int sdp_gen_record_pdu(const sdp_record_t *rec, sdp_buf_t *pdu);
-int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *seqlen);
+int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *seqlen, int bytesleft);
-sdp_data_t *sdp_extract_attr(const uint8_t *pdata, int *extractedLength, sdp_record_t *rec);
+sdp_data_t *sdp_extract_attr(const uint8_t *pdata, int *extractedLength, sdp_record_t *rec, int bytesleft);
void sdp_pattern_add_uuid(sdp_record_t *rec, uuid_t *uuid);
void sdp_pattern_add_uuidseq(sdp_record_t *rec, sdp_list_t *seq);
diff -ur bluetooth.orig/bluez-libs-3.30/src/sdp.c bluetooth/bluez-libs-3.30/src/sdp.c
--- bluetooth.orig/bluez-libs-3.30/src/sdp.c 2008-06-13 13:36:13.990281000 -0700
+++ bluetooth/bluez-libs-3.30/src/sdp.c 2008-06-13 12:34:14.999932000 -0700
@@ -904,43 +904,81 @@
free(d);
}
-static sdp_data_t *extract_int(const void *p, int *len)
+static sdp_data_t *extract_int(const void *p, int *len, int bytesleft)
{
sdp_data_t *d = malloc(sizeof(sdp_data_t));
SDPDBG("Extracting integer\n");
memset(d, 0, sizeof(sdp_data_t));
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
+
d->dtd = *(uint8_t *) p;
p += sizeof(uint8_t);
*len += sizeof(uint8_t);
-
+ bytesleft -= sizeof(uint8_t);
+
switch (d->dtd) {
case SDP_DATA_NIL:
break;
case SDP_BOOL:
case SDP_INT8:
case SDP_UINT8:
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
d->val.uint8 = *(uint8_t *) p;
break;
case SDP_INT16:
case SDP_UINT16:
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
d->val.uint16 = ntohs(bt_get_unaligned((uint16_t *) p));
break;
case SDP_INT32:
case SDP_UINT32:
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
d->val.uint32 = ntohl(bt_get_unaligned((uint32_t *) p));
break;
case SDP_INT64:
case SDP_UINT64:
+ if (bytesleft < sizeof(uint64_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint64_t);
+ bytesleft -= sizeof(uint64_t);
d->val.uint64 = ntoh64(bt_get_unaligned((uint64_t *) p));
break;
case SDP_INT128:
case SDP_UINT128:
+ if (bytesleft < sizeof(uint128_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
*len += sizeof(uint128_t);
+ bytesleft -= sizeof(uint128_t);
ntoh128((uint128_t *) p, &d->val.uint128);
break;
default:
@@ -950,16 +988,21 @@
return d;
}
-static sdp_data_t *extract_uuid(const uint8_t *p, int *len, sdp_record_t *rec)
+static sdp_data_t *extract_uuid(const uint8_t *p, int *len, sdp_record_t *rec, int bytesleft)
{
sdp_data_t *d = malloc(sizeof(sdp_data_t));
SDPDBG("Extracting UUID");
memset(d, 0, sizeof(sdp_data_t));
- if (sdp_uuid_extract(p, &d->val.uuid, len) < 0) {
+ if (sdp_uuid_extract(p, &d->val.uuid, len, bytesleft) < 0) {
free(d);
return NULL;
}
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
d->dtd = *(uint8_t *) p;
if (rec)
sdp_pattern_add_uuid(rec, &d->val.uuid);
@@ -969,29 +1012,63 @@
/*
* Extract strings from the PDU (could be service description and similar info)
*/
-static sdp_data_t *extract_str(const void *p, int *len)
+static sdp_data_t *extract_str(const void *p, int *len, int bytesleft)
{
char *s;
int n;
sdp_data_t *d = malloc(sizeof(sdp_data_t));
memset(d, 0, sizeof(sdp_data_t));
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
+
d->dtd = *(uint8_t *) p;
p += sizeof(uint8_t);
*len += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
switch (d->dtd) {
case SDP_TEXT_STR8:
case SDP_URL_STR8:
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
n = *(uint8_t *) p;
p += sizeof(uint8_t);
- *len += sizeof(uint8_t) + n;
+ *len += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
+ if (n <= 0 || n > bytesleft) {
+ SDPERR("Invalid length of text string");
+ free(d);
+ return 0;
+ }
+ *len += n;
+ bytesleft -= n;
break;
case SDP_TEXT_STR16:
case SDP_URL_STR16:
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
n = ntohs(bt_get_unaligned((uint16_t *) p));
p += sizeof(uint16_t);
- *len += sizeof(uint16_t) + n;
+ *len += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
+ if (n <= 0 || n > bytesleft) {
+ SDPERR("Invalid length of text string");
+ free(d);
+ return 0;
+ }
+ *len += n;
+ bytesleft -= n;
break;
default:
SDPERR("Sizeof text string > UINT16_MAX\n");
@@ -1000,6 +1077,12 @@
}
s = malloc(n + 1);
+ if (NULL == s) {
+ SDPERR("Cannot allocate memory for string");
+ free(d);
+ return 0;
+ }
+
memset(s, 0, n + 1);
memcpy(s, p, n);
@@ -1011,7 +1094,7 @@
return d;
}
-static sdp_data_t *extract_seq(const void *p, int *len, sdp_record_t *rec)
+static sdp_data_t *extract_seq(const void *p, int *len, sdp_record_t *rec, int bytesleft)
{
int seqlen, n = 0;
sdp_data_t *curr, *prev;
@@ -1019,17 +1102,24 @@
SDPDBG("Extracting SEQ");
memset(d, 0, sizeof(sdp_data_t));
- *len = sdp_extract_seqtype(p, &d->dtd, &seqlen);
+ *len = sdp_extract_seqtype(p, &d->dtd, &seqlen, bytesleft);
SDPDBG("Sequence Type : 0x%x length : 0x%x\n", d->dtd, seqlen);
if (*len == 0)
return d;
+ if (bytesleft < *len) {
+ SDPERR("Unexpected end of packet");
+ free(d);
+ return 0;
+ }
+
p += *len;
+ bytesleft -= *len;
curr = prev = NULL;
while (n < seqlen) {
int attrlen = 0;
- curr = sdp_extract_attr(p, &attrlen, rec);
+ curr = sdp_extract_attr(p, &attrlen, rec, bytesleft);
if (curr == NULL)
break;
@@ -1040,6 +1130,7 @@
prev = curr;
p += attrlen;
n += attrlen;
+ bytesleft -= attrlen;
SDPDBG("Extracted: %d SequenceLength: %d", n, seqlen);
}
@@ -1048,10 +1139,16 @@
return d;
}
-sdp_data_t *sdp_extract_attr(const uint8_t *p, int *size, sdp_record_t *rec)
+sdp_data_t *sdp_extract_attr(const uint8_t *p, int *size, sdp_record_t *rec, int bytesleft)
{
sdp_data_t *elem;
int n = 0;
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
+
uint8_t dtd = *(const uint8_t *)p;
SDPDBG("extract_attr: dtd=0x%x", dtd);
@@ -1068,12 +1165,12 @@
case SDP_INT32:
case SDP_INT64:
case SDP_INT128:
- elem = extract_int(p, &n);
+ elem = extract_int(p, &n, bytesleft);
break;
case SDP_UUID16:
case SDP_UUID32:
case SDP_UUID128:
- elem = extract_uuid(p, &n, rec);
+ elem = extract_uuid(p, &n, rec, bytesleft);
break;
case SDP_TEXT_STR8:
case SDP_TEXT_STR16:
@@ -1081,7 +1178,7 @@
case SDP_URL_STR8:
case SDP_URL_STR16:
case SDP_URL_STR32:
- elem = extract_str(p, &n);
+ elem = extract_str(p, &n, bytesleft);
break;
case SDP_SEQ8:
case SDP_SEQ16:
@@ -1089,13 +1186,14 @@
case SDP_ALT8:
case SDP_ALT16:
case SDP_ALT32:
- elem = extract_seq(p, &n, rec);
+ elem = extract_seq(p, &n, rec, bytesleft);
break;
default:
SDPERR("Unknown data descriptor : 0x%x terminating\n", dtd);
return NULL;
}
*size += n;
+ bytesleft -= n;
return elem;
}
@@ -1122,21 +1220,37 @@
}
#endif
-sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int *scanned)
+sdp_record_t *sdp_extract_pdu(const uint8_t *buf, int *scanned, int packet_size)
{
int extracted = 0, seqlen = 0;
uint8_t dtd;
uint16_t attr;
sdp_record_t *rec = sdp_record_alloc();
const uint8_t *p = buf;
+ int bytesleft = packet_size;
- *scanned = sdp_extract_seqtype(buf, &dtd, &seqlen);
+ *scanned = sdp_extract_seqtype(buf, &dtd, &seqlen, bytesleft);
+ if (*scanned == 0) {
+ SDPERR("Sequence length 0 of incoming SDP PDU");
+ return rec;
+ }
p += *scanned;
+ bytesleft -= *scanned;
+ if (bytesleft < seqlen) {
+ SDPERR("Insufficient data in packet to contain sequence");
+ return rec;
+ }
+
rec->attrlist = NULL;
while (extracted < seqlen) {
int n = sizeof(uint8_t), attrlen = 0;
sdp_data_t *data = NULL;
+ if (bytesleft < sizeof(uint8_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ break;
+ }
+
SDPDBG("Extract PDU, sequenceLength: %d localExtractedLength: %d", seqlen, extracted);
dtd = *(uint8_t *) p;
attr = ntohs(bt_get_unaligned((uint16_t *) (p + n)));
@@ -1144,7 +1258,8 @@
SDPDBG("DTD of attrId : %d Attr id : 0x%x \n", dtd, attr);
- data = sdp_extract_attr(p + n, &attrlen, rec);
+ bytesleft -= sizeof(uint8_t)+sizeof(uint16_t);
+ data = sdp_extract_attr(p + n, &attrlen, rec, bytesleft);
SDPDBG("Attr id : 0x%x attrValueLength : %d\n", attr, attrlen);
@@ -1162,6 +1277,7 @@
extracted += n;
p += n;
+ bytesleft -= n;
sdp_attr_replace(rec, attr, data);
SDPDBG("Extract PDU, seqLength: %d localExtractedLength: %d",
seqlen, extracted);
@@ -1266,26 +1382,43 @@
* Extract the sequence type and its length, and return offset into buf
* or 0 on failure.
*/
-int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *size)
+int sdp_extract_seqtype(const uint8_t *buf, uint8_t *dtdp, int *size, int bytesleft)
{
uint8_t dtd = *(uint8_t *) buf;
int scanned = sizeof(uint8_t);
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
buf += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
*dtdp = dtd;
switch (dtd) {
case SDP_SEQ8:
case SDP_ALT8:
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
*size = *(uint8_t *) buf;
scanned += sizeof(uint8_t);
break;
case SDP_SEQ16:
case SDP_ALT16:
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
*size = ntohs(bt_get_unaligned((uint16_t *) buf));
scanned += sizeof(uint16_t);
break;
case SDP_SEQ32:
case SDP_ALT32:
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ return 0;
+ }
*size = ntohl(bt_get_unaligned((uint32_t *) buf));
scanned += sizeof(uint32_t);
break;
@@ -2329,9 +2462,16 @@
return 0;
}
-int sdp_uuid_extract(const uint8_t *p, uuid_t *uuid, int *scanned)
+int sdp_uuid_extract(const uint8_t *p, uuid_t *uuid, int *scanned, int bytesleft)
{
- uint8_t type = *(const uint8_t *) p;
+ uint8_t type;
+
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
+
+ type = *(const uint8_t *) p;
if (!SDP_IS_UUID(type)) {
SDPERR("Unknown data type : %d expecting a svc UUID\n", type);
@@ -2339,17 +2479,34 @@
}
p += sizeof(uint8_t);
*scanned += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
+
if (type == SDP_UUID16) {
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
sdp_uuid16_create(uuid, ntohs(bt_get_unaligned((uint16_t *) p)));
*scanned += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
p += sizeof(uint16_t);
} else if (type == SDP_UUID32) {
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
sdp_uuid32_create(uuid, ntohl(bt_get_unaligned((uint32_t *) p)));
*scanned += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
p += sizeof(uint32_t);
} else {
+ if (bytesleft < sizeof(uint128_t)) {
+ SDPERR("Unexpected end of packet");
+ return -1;
+ }
sdp_uuid128_create(uuid, p);
*scanned += sizeof(uint128_t);
+ bytesleft -= sizeof(uint128_t);
p += sizeof(uint128_t);
}
return 0;
@@ -2447,6 +2604,7 @@
uint32_t reqsize, rspsize;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
int status;
+ int bytesleft;
SDPDBG("");
@@ -2483,9 +2641,19 @@
status = sdp_send_req_w4_rsp(session, req, rsp, reqsize, &rspsize);
if (status < 0)
goto end;
+
+ bytesleft = rspsize;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+
rsphdr = (sdp_pdu_hdr_t *) rsp;
p = rsp + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
/* Invalid service record */
@@ -2495,7 +2663,13 @@
errno = EPROTO;
status = -1;
} else {
- if (handle)
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+ if (handle)
*handle = ntohl(bt_get_unaligned((uint32_t *) p));
}
@@ -2556,6 +2730,7 @@
uint32_t reqsize = 0, rspsize = 0;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
int status;
+ int bytesleft;
SDPDBG("");
@@ -2590,9 +2765,19 @@
if (status < 0)
goto end;
- rsphdr = (sdp_pdu_hdr_t *) rspbuf;
+ bytesleft = rspsize;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+
+ rsphdr = (sdp_pdu_hdr_t *) rspbuf;
p = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
status = bt_get_unaligned((uint16_t *) p);
+ bytesleft -= sizeof(uint16_t);
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
/* For this case the status always is invalid record handle */
@@ -2644,6 +2829,7 @@
uint32_t handle;
sdp_buf_t pdu;
int status;
+ int bytesleft;
SDPDBG("");
@@ -2688,12 +2874,23 @@
status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
if (status < 0)
goto end;
+
+ bytesleft = rspsize;
SDPDBG("Send req status : %d\n", status);
- rsphdr = (sdp_pdu_hdr_t *) rspbuf;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ errno = EINVAL;
+ status = -1;
+ goto end;
+ }
+
+ rsphdr = (sdp_pdu_hdr_t *) rspbuf;
p = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
status = bt_get_unaligned((uint16_t *) p);
+ bytesleft -= sizeof(uint16_t);
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
/* The status can be invalid sintax or invalid record handle */
@@ -2764,18 +2961,23 @@
* handles are not in "data element sequence" form, but just like
* an array of service handles
*/
-static void extract_record_handle_seq(uint8_t *pdu, sdp_list_t **seq, int count, int *scanned)
+static void extract_record_handle_seq(uint8_t *pdu, sdp_list_t **seq, int count, int *scanned, int bytesleft)
{
sdp_list_t *pSeq = *seq;
uint8_t *pdata = pdu;
int n;
for (n = 0; n < count; n++) {
+ if (bytesleft < sizeof(uint32_t)) {
+ SDPERR("Unexpected end of packet");
+ break;
+ }
uint32_t *pSvcRec = malloc(sizeof(uint32_t));
*pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
pSeq = sdp_list_append(pSeq, pSvcRec);
pdata += sizeof(uint32_t);
*scanned += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
}
*seq = pSeq;
}
@@ -2843,12 +3045,17 @@
unsigned char data[16];
} __attribute__ ((packed)) sdp_cstate_t;
-static int copy_cstate(uint8_t *pdata, const sdp_cstate_t *cstate)
+static int copy_cstate(uint8_t *pdata, int pdata_len, const sdp_cstate_t *cstate)
{
if (cstate) {
- *pdata++ = cstate->length;
- memcpy(pdata, cstate->data, cstate->length);
- return cstate->length + 1;
+ uint8_t len = cstate->length;
+ if ((int)len > pdata_len) {
+ SDPERR("Continuation state size exceeds internal buffer");
+ } else {
+ *pdata++ = len;
+ memcpy(pdata, cstate->data, len);
+ return len + 1;
+ }
}
*pdata = 0;
return 1;
@@ -2892,9 +3099,11 @@
int seqlen = 0;
int scanned, total_rec_count, rec_count;
uint8_t *pdata, *_pdata;
+ int _pdata_len;
uint8_t *reqbuf, *rspbuf;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
sdp_cstate_t *cstate = NULL;
+ int bytesleft;
reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
@@ -2924,11 +3133,12 @@
_reqsize = reqsize;
_pdata = pdata;
+ _pdata_len = SDP_REQ_BUFFER_SIZE - reqsize;
*rsp = NULL;
do {
// Add continuation state or NULL (first time)
- reqsize = _reqsize + copy_cstate(_pdata, cstate);
+ reqsize = _reqsize + copy_cstate(_pdata, _pdata_len, cstate);
// Set the request header's param length
reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
@@ -2940,8 +3150,16 @@
*/
status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
if (status < 0)
- goto end;
-
+ goto end;
+
+ bytesleft = rspsize;
+
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
rsplen = 0;
rsphdr = (sdp_pdu_hdr_t *) rspbuf;
rsplen = ntohs(rsphdr->plen);
@@ -2953,14 +3171,22 @@
}
scanned = 0;
pdata = rspbuf + sizeof(sdp_pdu_hdr_t);
-
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
+ if (bytesleft < sizeof(uint16_t)+sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
// net service record match count
total_rec_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
pdata += sizeof(uint16_t);
scanned += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
rec_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
pdata += sizeof(uint16_t);
scanned += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
SDPDBG("Total svc count: %d\n", total_rec_count);
SDPDBG("Current svc count: %d\n", rec_count);
@@ -2970,12 +3196,19 @@
status = -1;
goto end;
}
- extract_record_handle_seq(pdata, rsp, rec_count, &scanned);
+ extract_record_handle_seq(pdata, rsp, rec_count, &scanned, bytesleft);
SDPDBG("BytesScanned : %d\n", scanned);
+ bytesleft -= scanned;
if (rsplen > scanned) {
uint8_t cstate_len;
+ if (bytesleft < sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet: continuation state data missing");
+ status = -1;
+ goto end;
+ }
+
pdata = rspbuf + sizeof(sdp_pdu_hdr_t) + scanned;
cstate_len = *(uint8_t *) pdata;
if (cstate_len > 0) {
@@ -3036,12 +3269,14 @@
int attr_list_len = 0;
int seqlen = 0;
uint8_t *pdata, *_pdata;
+ int _pdata_len;
uint8_t *reqbuf, *rspbuf;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
sdp_cstate_t *cstate = NULL;
uint8_t cstate_len = 0;
sdp_buf_t rsp_concat_buf;
sdp_record_t *rec = 0;
+ int bytesleft;
if (reqtype != SDP_ATTR_REQ_INDIVIDUAL && reqtype != SDP_ATTR_REQ_RANGE) {
errno = EINVAL;
@@ -3087,10 +3322,11 @@
// save before Continuation State
_pdata = pdata;
_reqsize = reqsize;
+ _pdata_len = SDP_REQ_BUFFER_SIZE - reqsize;
do {
// add NULL continuation state
- reqsize = _reqsize + copy_cstate(_pdata, cstate);
+ reqsize = _reqsize + copy_cstate(_pdata, _pdata_len, cstate);
// set the request header's param length
reqhdr->tid = htons(sdp_gen_tid(session));
@@ -3099,6 +3335,14 @@
status = sdp_send_req_w4_rsp(session, reqbuf, rspbuf, reqsize, &rspsize);
if (status < 0)
goto end;
+ bytesleft = rspsize;
+
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
rsp_count = 0;
rsphdr = (sdp_pdu_hdr_t *) rspbuf;
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
@@ -3107,12 +3351,25 @@
goto end;
}
pdata = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
+
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
attr_list_len += rsp_count;
pdata += sizeof(uint16_t);
+ bytesleft -= sizeof(uint16_t);
// if continuation state set need to re-issue request before parsing
- cstate_len = *(uint8_t *) (pdata + rsp_count);
+ if (bytesleft < rsp_count + sizeof(uint8_t)) {
+ SDPERR("Unexpected end of packet: continuation state data missing");
+ status = -1;
+ goto end;
+ }
+ cstate_len = *(uint8_t *) (pdata + rsp_count);
SDPDBG("Response id : %d\n", rsphdr->pdu_id);
SDPDBG("Attrlist byte count : %d\n", rsp_count);
@@ -3138,9 +3395,11 @@
if (attr_list_len > 0) {
int scanned = 0;
- if (rsp_concat_buf.data_size != 0)
+ if (rsp_concat_buf.data_size != 0) {
pdata = rsp_concat_buf.data;
- rec = sdp_extract_pdu(pdata, &scanned);
+ bytesleft = rsp_concat_buf.data_size;
+ }
+ rec = sdp_extract_pdu(pdata, &scanned, bytesleft);
if (!rec)
status = -1;
@@ -3269,6 +3528,7 @@
sdp_pdu_hdr_t *reqhdr;
uint8_t *pdata;
int cstate_len, seqlen = 0;
+ int pdata_len;
if (!session || !session->priv)
return -1;
@@ -3296,6 +3556,7 @@
// generate PDU
pdata = t->reqbuf + sizeof(sdp_pdu_hdr_t);
t->reqsize = sizeof(sdp_pdu_hdr_t);
+ pdata_len = SDP_REQ_BUFFER_SIZE - t->reqsize;
// add service class IDs for search
seqlen = gen_searchseq_pdu(pdata, search);
@@ -3305,13 +3566,15 @@
// now set the length and increment the pointer
t->reqsize += seqlen;
pdata += seqlen;
+ pdata_len -= seqlen;
bt_put_unaligned(htons(max_rec_num), (uint16_t *) pdata);
t->reqsize += sizeof(uint16_t);
pdata += sizeof(uint16_t);
+ pdata_len -= sizeof(uint16_t);
// set the request header's param length
- cstate_len = copy_cstate(pdata, NULL);
+ cstate_len = copy_cstate(pdata, pdata_len, NULL);
reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
@@ -3373,6 +3636,7 @@
sdp_pdu_hdr_t *reqhdr;
uint8_t *pdata;
int cstate_len, seqlen = 0;
+ int pdata_len;
if (!session || !session->priv)
return -1;
@@ -3400,16 +3664,19 @@
// generate PDU
pdata = t->reqbuf + sizeof(sdp_pdu_hdr_t);
t->reqsize = sizeof(sdp_pdu_hdr_t);
+ pdata_len = SDP_REQ_BUFFER_SIZE - t->reqsize;
// add the service record handle
bt_put_unaligned(htonl(handle), (uint32_t *) pdata);
t->reqsize += sizeof(uint32_t);
pdata += sizeof(uint32_t);
+ pdata_len -= sizeof(uint32_t);
// specify the response limit
bt_put_unaligned(htons(65535), (uint16_t *) pdata);
t->reqsize += sizeof(uint16_t);
pdata += sizeof(uint16_t);
+ pdata_len -= sizeof(uint16_t);
// get attr seq PDU form
seqlen = gen_attridseq_pdu(pdata, attrid_list,
@@ -3422,10 +3689,11 @@
// now set the length and increment the pointer
t->reqsize += seqlen;
pdata += seqlen;
+ pdata_len -= seqlen;
SDPDBG("Attr list length : %d\n", seqlen);
// set the request header's param length
- cstate_len = copy_cstate(pdata, NULL);
+ cstate_len = copy_cstate(pdata, pdata_len, NULL);
reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
@@ -3488,6 +3756,7 @@
sdp_pdu_hdr_t *reqhdr;
uint8_t *pdata;
int cstate_len, seqlen = 0;
+ int pdata_len;
if (!session || !session->priv)
return -1;
@@ -3515,6 +3784,7 @@
// generate PDU
pdata = t->reqbuf + sizeof(sdp_pdu_hdr_t);
t->reqsize = sizeof(sdp_pdu_hdr_t);
+ pdata_len = SDP_REQ_BUFFER_SIZE - t->reqsize;
// add service class IDs for search
seqlen = gen_searchseq_pdu(pdata, search);
@@ -3524,10 +3794,12 @@
// now set the length and increment the pointer
t->reqsize += seqlen;
pdata += seqlen;
+ pdata_len -= seqlen;
bt_put_unaligned(htons(SDP_MAX_ATTR_LEN), (uint16_t *) pdata);
t->reqsize += sizeof(uint16_t);
pdata += sizeof(uint16_t);
+ pdata_len -= sizeof(uint16_t);
SDPDBG("Max attr byte count : %d\n", SDP_MAX_ATTR_LEN);
@@ -3540,11 +3812,12 @@
}
pdata += seqlen;
+ pdata_len -= seqlen;
SDPDBG("Attr list length : %d\n", seqlen);
t->reqsize += seqlen;
// set the request header's param length
- cstate_len = copy_cstate(pdata, NULL);
+ cstate_len = copy_cstate(pdata, pdata_len, NULL);
reqhdr->plen = htons((t->reqsize + cstate_len) - sizeof(sdp_pdu_hdr_t));
if (sdp_send_req(session, t->reqbuf, t->reqsize + cstate_len) < 0) {
@@ -3757,7 +4030,7 @@
reqhdr->tid = htons(sdp_gen_tid(session));
// add continuation state
- cstate_len = copy_cstate(t->reqbuf + t->reqsize, pcstate);
+ cstate_len = copy_cstate(t->reqbuf + t->reqsize, SDP_REQ_BUFFER_SIZE - t->reqsize, pcstate);
reqsize = t->reqsize + cstate_len;
@@ -3838,12 +4111,14 @@
int seqlen = 0, attr_list_len = 0;
int rsp_count = 0, cstate_len = 0;
uint8_t *pdata, *_pdata;
+ int _pdata_len;
uint8_t *reqbuf, *rspbuf;
sdp_pdu_hdr_t *reqhdr, *rsphdr;
uint8_t dataType;
sdp_list_t *rec_list = NULL;
sdp_buf_t rsp_concat_buf;
sdp_cstate_t *cstate = NULL;
+ int bytesleft;
if (reqtype != SDP_ATTR_REQ_INDIVIDUAL && reqtype != SDP_ATTR_REQ_RANGE) {
errno = EINVAL;
@@ -3895,12 +4170,13 @@
// save before Continuation State
_pdata = pdata;
_reqsize = reqsize;
+ _pdata_len = SDP_REQ_BUFFER_SIZE - reqsize;
do {
reqhdr->tid = htons(sdp_gen_tid(session));
// add continuation state (can be null)
- reqsize = _reqsize + copy_cstate(_pdata, cstate);
+ reqsize = _reqsize + copy_cstate(_pdata, _pdata_len, cstate);
// set the request header's param length
reqhdr->plen = htons(reqsize - sizeof(sdp_pdu_hdr_t));
@@ -3910,6 +4186,13 @@
SDPDBG("Status : 0x%x\n", rsphdr->pdu_id);
goto end;
}
+
+ bytesleft = rspsize;
+ if (bytesleft < sizeof(sdp_pdu_hdr_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
if (rsphdr->pdu_id == SDP_ERROR_RSP) {
status = -1;
@@ -3917,9 +4200,25 @@
}
pdata = rspbuf + sizeof(sdp_pdu_hdr_t);
+ bytesleft -= sizeof(sdp_pdu_hdr_t);
+
+ if (bytesleft < sizeof(uint16_t)) {
+ SDPERR("Unexpected end of packet");
+ status = -1;
+ goto end;
+ }
+
rsp_count = ntohs(bt_get_unaligned((uint16_t *) pdata));
attr_list_len += rsp_count;
pdata += sizeof(uint16_t); // pdata points to attribute list
+ bytesleft -= sizeof(uint16_t);
+
+ if (bytesleft < rsp_count) {
+ SDPERR("Unexpected end of packet: continuation state data missing");
+ status = -1;
+ goto end;
+ }
+
cstate_len = *(uint8_t *) (pdata + rsp_count);
SDPDBG("Attrlist byte count : %d\n", attr_list_len);
@@ -3946,24 +4245,27 @@
if (attr_list_len > 0) {
int scanned = 0;
- if (rsp_concat_buf.data_size != 0)
+ if (rsp_concat_buf.data_size != 0) {
pdata = rsp_concat_buf.data;
+ bytesleft = rsp_concat_buf.data_size;
+ }
/*
* Response is a sequence of sequence(s) for one or
* more data element sequence(s) representing services
* for which attributes are returned
*/
- scanned = sdp_extract_seqtype(pdata, &dataType, &seqlen);
+ scanned = sdp_extract_seqtype(pdata, &dataType, &seqlen, bytesleft);
SDPDBG("Bytes scanned : %d\n", scanned);
SDPDBG("Seq length : %d\n", seqlen);
if (scanned && seqlen) {
pdata += scanned;
+ bytesleft -= scanned;
do {
int recsize = 0;
- sdp_record_t *rec = sdp_extract_pdu(pdata, &recsize);
+ sdp_record_t *rec = sdp_extract_pdu(pdata, &recsize, bytesleft);
if (rec == NULL) {
SDPERR("SVC REC is null\n");
status = -1;
@@ -3975,6 +4277,7 @@
}
scanned += recsize;
pdata += recsize;
+ bytesleft -= recsize;
SDPDBG("Loc seq length : %d\n", recsize);
SDPDBG("Svc Rec Handle : 0x%x\n", rec->handle);
diff -ur bluetooth.orig/bluez-utils-3.30/audio/headset.c bluetooth/bluez-utils-3.30/audio/headset.c
--- bluetooth.orig/bluez-utils-3.30/audio/headset.c 2008-06-13 13:36:14.201284000 -0700
+++ bluetooth/bluez-utils-3.30/audio/headset.c 2008-06-13 12:37:15.957158000 -0700
@@ -848,7 +848,7 @@
goto failed_not_supported;
}
- record = sdp_extract_pdu(array, &record_len);
+ record = sdp_extract_pdu(array, &record_len, array_len);
if (!record) {
error("Unable to extract service record from reply");
goto failed_not_supported;
diff -ur bluetooth.orig/bluez-utils-3.30/audio/manager.c bluetooth/bluez-utils-3.30/audio/manager.c
--- bluetooth.orig/bluez-utils-3.30/audio/manager.c 2008-06-13 13:36:14.204281000 -0700
+++ bluetooth/bluez-utils-3.30/audio/manager.c 2008-06-13 12:37:44.757153000 -0700
@@ -419,7 +419,7 @@
goto failed;
}
- record = sdp_extract_pdu(array, &record_len);
+ record = sdp_extract_pdu(array, &record_len, array_len);
if (!record) {
error("Unable to extract service record from reply");
goto done;
diff -ur bluetooth.orig/bluez-utils-3.30/common/glib-helper.c bluetooth/bluez-utils-3.30/common/glib-helper.c
--- bluetooth.orig/bluez-utils-3.30/common/glib-helper.c 2008-06-13 13:36:14.300310000 -0700
+++ bluetooth/bluez-utils-3.30/common/glib-helper.c 2008-06-13 12:25:08.683864000 -0700
@@ -62,23 +62,25 @@
int scanned, seqlen = 0;
uint8_t dataType;
int err = 0;
+ int bytesleft = size;
if (status || type != SDP_SVC_SEARCH_ATTR_RSP) {
err = -EPROTO;
goto done;
}
- scanned = sdp_extract_seqtype(rsp, &dataType, &seqlen);
+ scanned = sdp_extract_seqtype(rsp, &dataType, &seqlen, bytesleft);
if (!scanned || !seqlen)
goto done;
rsp += scanned;
+ bytesleft -= scanned;
do {
sdp_record_t *rec;
int recsize;
recsize = 0;
- rec = sdp_extract_pdu(rsp, &recsize);
+ rec = sdp_extract_pdu(rsp, &recsize, bytesleft);
if (!rec)
break;
@@ -89,6 +91,7 @@
scanned += recsize;
rsp += recsize;
+ bytesleft -= recsize;
recs = sdp_list_append(recs, rec);
} while (scanned < size);
diff -ur bluetooth.orig/bluez-utils-3.30/hcid/dbus-database.c bluetooth/bluez-utils-3.30/hcid/dbus-database.c
--- bluetooth.orig/bluez-utils-3.30/hcid/dbus-database.c 2008-06-13 13:36:14.708148000 -0700
+++ bluetooth/bluez-utils-3.30/hcid/dbus-database.c 2008-06-13 12:27:57.935715000 -0700
@@ -107,7 +107,7 @@
if (len <= 0)
return error_invalid_arguments(conn, msg, NULL);
- sdp_record = sdp_extract_pdu(record, &scanned);
+ sdp_record = sdp_extract_pdu(record, &scanned, len);
if (!sdp_record) {
error("Parsing of service record failed");
return error_failed_errno(conn, msg, EIO);
@@ -253,7 +253,7 @@
if (!user_record)
return error_not_available(conn, msg);
- sdp_record = sdp_extract_pdu(bin_record, &scanned);
+ sdp_record = sdp_extract_pdu(bin_record, &scanned, size);
if (!sdp_record) {
error("Parsing of service record failed");
return error_invalid_arguments(conn, msg, NULL);
diff -ur bluetooth.orig/bluez-utils-3.30/hcid/dbus-sdp.c bluetooth/bluez-utils-3.30/hcid/dbus-sdp.c
--- bluetooth.orig/bluez-utils-3.30/hcid/dbus-sdp.c 2008-06-13 13:36:14.711144000 -0700
+++ bluetooth/bluez-utils-3.30/hcid/dbus-sdp.c 2008-06-13 13:25:14.830806000 -0700
@@ -494,7 +494,7 @@
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
DBUS_TYPE_BYTE_AS_STRING, &array_iter);
- rec = sdp_extract_pdu(rsp, &scanned);
+ rec = sdp_extract_pdu(rsp, &scanned, size);
if (rec == NULL || size != scanned) {
error("Invalid service record!");
goto done;
@@ -563,7 +563,7 @@
reply = dbus_message_new_method_return(ctxt->rq);
- rec = sdp_extract_pdu(rsp, &scanned);
+ rec = sdp_extract_pdu(rsp, &scanned, size);
if (rec == NULL || size != scanned) {
error("Invalid service record!");
goto done;
@@ -731,6 +731,7 @@
GSList *l = NULL;
int scanned, extracted = 0, len = 0, recsize = 0;
uint8_t dtd = 0;
+ int bytesleft = (int)size;
if (!ctxt)
return;
@@ -766,16 +767,18 @@
DBUS_TYPE_STRING, &dst,
DBUS_TYPE_INVALID);
- scanned = sdp_extract_seqtype(rsp, &dtd, &len);
+ scanned = sdp_extract_seqtype(rsp, &dtd, &len, bytesleft);
rsp += scanned;
+ bytesleft -= scanned;
for (; extracted < len; rsp += recsize, extracted += recsize) {
sdp_record_t *rec;
sdp_data_t *d;
recsize = 0;
- rec = sdp_extract_pdu(rsp, &recsize);
+ rec = sdp_extract_pdu(rsp, &recsize, bytesleft);
if (!rec)
break;
+ bytesleft -= recsize;
sdp_store_record(src, dst, rec->handle, rsp, recsize);
diff -ur bluetooth.orig/bluez-utils-3.30/input/manager.c bluetooth/bluez-utils-3.30/input/manager.c
--- bluetooth.orig/bluez-utils-3.30/input/manager.c 2008-06-13 13:36:15.400156000 -0700
+++ bluetooth/bluez-utils-3.30/input/manager.c 2008-06-13 12:39:37.109807000 -0700
@@ -536,7 +536,7 @@
goto fail;
}
- pr->hid_rec = sdp_extract_pdu(rec_bin, &scanned);
+ pr->hid_rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (!pr->hid_rec) {
error_not_supported(pr->conn, pr->msg);
goto fail;
@@ -673,7 +673,7 @@
goto fail;
}
- pr->pnp_rec = sdp_extract_pdu(rec_bin, &scanned);
+ pr->pnp_rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (get_handles(pr, hid_uuid, hid_handle_reply) < 0) {
error_not_supported(pr->conn, pr->msg);
error("HID service search request failed");
@@ -793,7 +793,7 @@
goto fail;
}
- rec = sdp_extract_pdu(rec_bin, &scanned);
+ rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (!rec) {
error_not_supported(pr->conn, pr->msg);
goto fail;
diff -ur bluetooth.orig/bluez-utils-3.30/network/manager.c bluetooth/bluez-utils-3.30/network/manager.c
--- bluetooth.orig/bluez-utils-3.30/network/manager.c 2008-06-13 13:36:15.503140000 -0700
+++ bluetooth/bluez-utils-3.30/network/manager.c 2008-06-13 12:40:13.287871000 -0700
@@ -253,7 +253,7 @@
goto fail;
}
- rec = sdp_extract_pdu(rec_bin, &scanned);
+ rec = sdp_extract_pdu(rec_bin, &scanned, len);
/* Concat remote name and service name */
memset(name, 0, MAX_NAME_SIZE);
diff -ur bluetooth.orig/bluez-utils-3.30/sdpd/request.c bluetooth/bluez-utils-3.30/sdpd/request.c
--- bluetooth.orig/bluez-utils-3.30/sdpd/request.c 2008-06-13 13:36:15.720000000 -0700
+++ bluetooth/bluez-utils-3.30/sdpd/request.c 2008-06-13 13:30:53.543764000 -0700
@@ -67,8 +67,9 @@
uint8_t dataType;
int status = 0;
const uint8_t *p;
+ int bytesleft = len;
- scanned = sdp_extract_seqtype(buf, &seqType, &data_size);
+ scanned = sdp_extract_seqtype(buf, &seqType, &data_size, bytesleft);
debug("Seq type : %d", seqType);
if (!scanned || (seqType != SDP_SEQ8 && seqType != SDP_SEQ16)) {
@@ -76,12 +77,18 @@
return -1;
}
p = buf + scanned;
+ bytesleft -= scanned;
debug("Data size : %d", data_size);
for (;;) {
char *pElem = NULL;
int localSeqLength = 0;
+
+ if (bytesleft < sizeof(uint8_t)) {
+ debug("->Unexpected end of buffer");
+ return -1;
+ }
dataType = *(uint8_t *)p;
debug("Data type: 0x%02x", dataType);
@@ -100,6 +107,12 @@
case SDP_UINT16:
p += sizeof(uint8_t);
seqlen += sizeof(uint8_t);
+ bytesleft -= sizeof(uint8_t);
+ if (bytesleft < sizeof(uint16_t)) {
+ debug("->Unexpected end of buffer");
+ return -1;
+ }
+
pElem = malloc(sizeof(uint16_t));
bt_put_unaligned(ntohs(bt_get_unaligned((uint16_t *)p)), (uint16_t *)pElem);
p += sizeof(uint16_t);
@@ -108,7 +121,13 @@
case SDP_UINT32:
p += sizeof(uint8_t);
seqlen += sizeof(uint8_t);
- pElem = malloc(sizeof(uint32_t));
+ bytesleft -= sizeof(uint8_t);
+ if (bytesleft < sizeof(uint32_t)) {
+ debug("->Unexpected end of buffer");
+ return -1;
+ }
+
+ pElem = malloc(sizeof(uint32_t));
bt_put_unaligned(ntohl(bt_get_unaligned((uint32_t *)p)), (uint32_t *)pElem);
p += sizeof(uint32_t);
seqlen += sizeof(uint32_t);
@@ -117,10 +136,11 @@
case SDP_UUID32:
case SDP_UUID128:
pElem = malloc(sizeof(uuid_t));
- status = sdp_uuid_extract(p, (uuid_t *)pElem, &localSeqLength);
+ status = sdp_uuid_extract(p, (uuid_t *)pElem, &localSeqLength, bytesleft);
if (status == 0) {
seqlen += localSeqLength;
p += localSeqLength;
+ bytesleft -= localSeqLength;
}
break;
default:
diff -ur bluetooth.orig/bluez-utils-3.30/sdpd/service.c bluetooth/bluez-utils-3.30/sdpd/service.c
--- bluetooth.orig/bluez-utils-3.30/sdpd/service.c 2008-06-13 13:36:15.716011000 -0700
+++ bluetooth/bluez-utils-3.30/sdpd/service.c 2008-06-13 12:53:10.911126000 -0700
@@ -412,7 +412,7 @@
}
// FIXME: refactor for server-side
-static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t handleExpected, int *scanned)
+static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t handleExpected, int *scanned, int bytesleft)
{
int extractStatus = -1, localExtractedLength = 0;
uint8_t dtd;
@@ -422,8 +422,9 @@
sdp_data_t *pAttr = NULL;
uint32_t handle = 0xffffffff;
- *scanned = sdp_extract_seqtype(p, &dtd, &seqlen);
+ *scanned = sdp_extract_seqtype(p, &dtd, &seqlen, bytesleft);
p += *scanned;
+ bytesleft -= *scanned;
lookAheadAttrId = ntohs(bt_get_unaligned((uint16_t *) (p + sizeof(uint8_t))));
debug("Look ahead attr id : %d", lookAheadAttrId);
@@ -464,7 +465,7 @@
debug("DTD of attrId : %d Attr id : 0x%x", dtd, attrId);
- pAttr = sdp_extract_attr(p + attrSize, &attrValueLength, rec);
+ pAttr = sdp_extract_attr(p + attrSize, &attrValueLength, rec, bytesleft - attrSize);
debug("Attr id : 0x%x attrValueLength : %d", attrId, attrValueLength);
@@ -475,6 +476,7 @@
}
localExtractedLength += attrSize;
p += attrSize;
+ bytesleft -= attrSize;
sdp_attr_replace(rec, attrId, pAttr);
extractStatus = 0;
debug("Extract PDU, seqLength: %d localExtractedLength: %d",
@@ -500,15 +502,17 @@
sdp_data_t *handle;
uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
sdp_record_t *rec;
+ int bytesleft = req->len - sizeof(sdp_pdu_hdr_t);
req->flags = *p++;
if (req->flags & SDP_DEVICE_RECORD) {
bacpy(&req->device, (bdaddr_t *) p);
p += sizeof(bdaddr_t);
+ bytesleft -= sizeof(bdaddr_t);
}
// save image of PDU: we need it when clients request this attribute
- rec = extract_pdu_server(&req->device, p, 0xffffffff, &scanned);
+ rec = extract_pdu_server(&req->device, p, 0xffffffff, &scanned, bytesleft);
if (!rec)
goto invalid;
@@ -578,18 +582,20 @@
sdp_record_t *orec;
int status = 0, scanned = 0;
uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
+ int bytesleft = req->len - sizeof(sdp_pdu_hdr_t);
uint32_t handle = ntohl(bt_get_unaligned((uint32_t *) p));
debug("Svc Rec Handle: 0x%x", handle);
p += sizeof(uint32_t);
+ bytesleft -= sizeof(uint32_t);
orec = sdp_record_find(handle);
debug("SvcRecOld: %p", orec);
if (orec) {
- sdp_record_t *nrec = extract_pdu_server(BDADDR_ANY, p, handle, &scanned);
+ sdp_record_t *nrec = extract_pdu_server(BDADDR_ANY, p, handle, &scanned, bytesleft);
if (nrec && handle == nrec->handle) {
update_db_timestamp();
update_svclass_list();
diff -ur bluetooth.orig/bluez-utils-3.30/serial/manager.c bluetooth/bluez-utils-3.30/serial/manager.c
--- bluetooth.orig/bluez-utils-3.30/serial/manager.c 2008-06-13 13:36:15.779001000 -0700
+++ bluetooth/bluez-utils-3.30/serial/manager.c 2008-06-13 12:40:48.550090000 -0700
@@ -520,7 +520,7 @@
goto fail;
}
- rec = sdp_extract_pdu(rec_bin, &scanned);
+ rec = sdp_extract_pdu(rec_bin, &scanned, len);
if (!rec) {
error("Can't extract SDP record.");
error_not_supported(pc->conn, pc->msg);
[-- Attachment #3: Type: text/plain, Size: 247 bytes --]
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
[-- Attachment #4: Type: text/plain, Size: 164 bytes --]
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bluez-devel] SDP payload processing vulnerability
2008-06-16 20:27 [Bluez-devel] SDP payload processing vulnerability Glenn Durfee
2008-06-16 20:49 ` Glenn Durfee
@ 2008-06-16 20:59 ` Marcel Holtmann
2008-06-16 21:21 ` Glenn Durfee
1 sibling, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2008-06-16 20:59 UTC (permalink / raw)
To: BlueZ development
Hi Glenn,
> The SDP parsing code blindly trusts string length fields in incoming
> SDP packets, exposing reliant applications to over-the-wireless memory
> manipulation attacks. An attacker need only send a malformed
> response to an SDP query to take advantage of this.
the SDP part has always been the weak point in every Bluetooth stack.
Our server side it pretty good, but it seems the client one is really
bad and I must admit that I never looked into in that in details.
> This is most apparent in file bluez-libs-3.30/src/sdp.c, lines 988,
> 994, 1002 (see below). Also elsewhere in the code where input
> pointers are advanced without checking bytes remaining to be parsed.
> The root of the problem is that in bluez-libs-3.30/src/sdp.c:1125, the
> function sdp_extract_pdu() takes a buffer to parse (in) and a pointer
> to a length field (out), but it does not take an incoming length field
> (in).
>
> Attached is a patch to fix this issue. Basically I added a
> "bytesleft" argument to all of the SDP payload processing routines;
> length fields are checked
> against the number of remaining bytes to ensure the parser doesn't run
> past the end of the packet, or do crazy things like malloc two gigs of
> memory. This touches a lot of places, and changes the external API
> for SDP payload processing, but I don't see any other way to do this
> -- the parser MUST be aware of the incoming packet size in order for
> this to be secure.
Changing the API is really a problem here. We can't do that. At least
not that easily. We can extend the API with more safe calls and then
slowly move over the clients.
Regards
Marcel
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bluez-devel] SDP payload processing vulnerability
2008-06-16 20:59 ` Marcel Holtmann
@ 2008-06-16 21:21 ` Glenn Durfee
2008-06-16 21:37 ` Marcel Holtmann
0 siblings, 1 reply; 5+ messages in thread
From: Glenn Durfee @ 2008-06-16 21:21 UTC (permalink / raw)
To: BlueZ development
Marcel,
Thank you for the quick reply.
> the SDP part has always been the weak point in every Bluetooth stack.
> Our server side it pretty good, but it seems the client one is really
> bad and I must admit that I never looked into in that in details.
I'm concerned that hcid is also vulnerable to this (which would make
it both a server and a client problem?).
> Changing the API is really a problem here. We can't do that. At least
> not that easily. We can extend the API with more safe calls and then
> slowly move over the clients.
That sounds good. I can write safe versions of the parsing routines
and move everything I can find over to the new API; old clients will
still work with the old (unsafe) API. This may present some
maintenance challenges since there will be 2x parsing code, but it is
better than leaving security holes everywhere.
Is bluez.org currently down? I can't seem to get at the latest version...
Glenn
On Mon, Jun 16, 2008 at 1:59 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Glenn,
>
>> The SDP parsing code blindly trusts string length fields in incoming
>> SDP packets, exposing reliant applications to over-the-wireless memory
>> manipulation attacks. An attacker need only send a malformed
>> response to an SDP query to take advantage of this.
>
> the SDP part has always been the weak point in every Bluetooth stack.
> Our server side it pretty good, but it seems the client one is really
> bad and I must admit that I never looked into in that in details.
>
>> This is most apparent in file bluez-libs-3.30/src/sdp.c, lines 988,
>> 994, 1002 (see below). Also elsewhere in the code where input
>> pointers are advanced without checking bytes remaining to be parsed.
>> The root of the problem is that in bluez-libs-3.30/src/sdp.c:1125, the
>> function sdp_extract_pdu() takes a buffer to parse (in) and a pointer
>> to a length field (out), but it does not take an incoming length field
>> (in).
>>
>> Attached is a patch to fix this issue. Basically I added a
>> "bytesleft" argument to all of the SDP payload processing routines;
>> length fields are checked
>> against the number of remaining bytes to ensure the parser doesn't run
>> past the end of the packet, or do crazy things like malloc two gigs of
>> memory. This touches a lot of places, and changes the external API
>> for SDP payload processing, but I don't see any other way to do this
>> -- the parser MUST be aware of the incoming packet size in order for
>> this to be secure.
>
> Changing the API is really a problem here. We can't do that. At least
> not that easily. We can extend the API with more safe calls and then
> slowly move over the clients.
>
> Regards
>
> Marcel
>
>
>
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://sourceforge.net/services/buy/index.php
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bluez-devel] SDP payload processing vulnerability
2008-06-16 21:21 ` Glenn Durfee
@ 2008-06-16 21:37 ` Marcel Holtmann
0 siblings, 0 replies; 5+ messages in thread
From: Marcel Holtmann @ 2008-06-16 21:37 UTC (permalink / raw)
To: BlueZ development
Hi Glenn,
> > the SDP part has always been the weak point in every Bluetooth stack.
> > Our server side it pretty good, but it seems the client one is really
> > bad and I must admit that I never looked into in that in details.
>
> I'm concerned that hcid is also vulnerable to this (which would make
> it both a server and a client problem?).
in theory it is, but you have to trigger a SDP client transaction first
and it is almost impossible to do this remotely. Yes, I can think of
tricks on how to do that, but that is besides the point here.
> > Changing the API is really a problem here. We can't do that. At least
> > not that easily. We can extend the API with more safe calls and then
> > slowly move over the clients.
>
> That sounds good. I can write safe versions of the parsing routines
> and move everything I can find over to the new API; old clients will
> still work with the old (unsafe) API. This may present some
> maintenance challenges since there will be 2x parsing code, but it is
> better than leaving security holes everywhere.
You can have the old API call the new one with a NULL parameter and yes,
it might be confusing, but it is better than breaking the API.
Please use the BlueZ coding style when doing the patch and have small
pieces. I am not going to review the whole think as once. Please send
small updates. It is faster this way.
> Is bluez.org currently down? I can't seem to get at the latest version...
Yeah. The server lost its routing information. I am working on it.
Use the CVS at bluez.sf.net or the GIT clone at git.infradead.org since
patches against the last release are outdated.
Regards
Marcel
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-06-16 21:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-16 20:27 [Bluez-devel] SDP payload processing vulnerability Glenn Durfee
2008-06-16 20:49 ` Glenn Durfee
2008-06-16 20:59 ` Marcel Holtmann
2008-06-16 21:21 ` Glenn Durfee
2008-06-16 21:37 ` Marcel Holtmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox