Attachment 310164 Details for Bug 452715 – Fixes for SDP vulnerability in bluez-utils

[patch] Fixes for SDP vulnerability in bluez-utils

patch-sdp-vuln-bluez-utils-3.33-to-3.34 (text/plain), 9.95 KB, created by Marcel Holtmann on 2008-06-24 16:38:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel Holtmann

Created: 2008-06-24 16:38:11 UTC

Size: 9.95 KB

patch

obsolete

>diff --git a/common/glib-helper.c b/common/glib-helper.c
>index eab30b5..931b99a 100644
>--- a/common/glib-helper.c
>+++ b/common/glib-helper.c
>@@ -115,7 +115,7 @@ static void search_completed_cb(uint8_t type, uint16_t status,
> {
> 	struct search_context *ctxt = user_data;
> 	sdp_list_t *recs = NULL;
>-	int scanned, seqlen = 0;
>+	int scanned, seqlen = 0, bytesleft = size;
> 	uint8_t dataType;
> 	int err = 0;
> 
>@@ -124,17 +124,18 @@ static void search_completed_cb(uint8_t type, uint16_t status,
> 		goto done;
> 	}
> 
>-	scanned = sdp_extract_seqtype(rsp, &dataType, &seqlen);
>+	scanned = sdp_extract_seqtype_safe(rsp, bytesleft, &dataType, &seqlen);
> 	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_safe(rsp, bytesleft, &recsize);
> 		if (!rec)
> 			break;
> 
>@@ -145,9 +146,10 @@ static void search_completed_cb(uint8_t type, uint16_t status,
> 
> 		scanned += recsize;
> 		rsp += recsize;
>+		bytesleft -= recsize;
> 
> 		recs = sdp_list_append(recs, rec);
>-	} while (scanned < size);
>+	} while (scanned < size && bytesleft > 0);
> 
> done:
> 	sdp_close(ctxt->session);
>diff --git a/hcid/dbus-database.c b/hcid/dbus-database.c
>index c6a689d..ab88c8f 100644
>--- a/hcid/dbus-database.c
>+++ b/hcid/dbus-database.c
>@@ -119,7 +119,7 @@ static DBusMessage *add_service_record(DBusConnection *conn,
> 	if (len <= 0)
> 		return invalid_arguments(msg);
> 
>-	sdp_record = sdp_extract_pdu(record, &scanned);
>+	sdp_record = sdp_extract_pdu_safe(record, len, &scanned);
> 	if (!sdp_record) {
> 		error("Parsing of service record failed");
> 		return failed(msg);
>@@ -263,7 +263,7 @@ static DBusMessage *update_service_record(DBusConnection *conn,
> 	if (!user_record)
> 		return not_available(msg);
> 
>-	sdp_record = sdp_extract_pdu(bin_record, &scanned);
>+	sdp_record = sdp_extract_pdu_safe(bin_record, size, &scanned);
> 	if (!sdp_record) {
> 		error("Parsing of service record failed");
> 		return invalid_arguments(msg);
>diff --git a/hcid/dbus-sdp.c b/hcid/dbus-sdp.c
>index cf018f1..441e823 100644
>--- a/hcid/dbus-sdp.c
>+++ b/hcid/dbus-sdp.c
>@@ -499,7 +499,7 @@ static void remote_svc_rec_completed_cb(uint8_t type, uint16_t err,
> 	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_safe(rsp, size, &scanned);
> 	if (rec == NULL || size != scanned) {
> 		error("Invalid service record!");
> 		goto done;
>@@ -562,7 +562,7 @@ static void remote_svc_rec_completed_xml_cb(uint8_t type, uint16_t err,
> 
> 	reply = dbus_message_new_method_return(ctxt->rq);
> 
>-	rec = sdp_extract_pdu(rsp, &scanned);
>+	rec = sdp_extract_pdu_safe(rsp, size, &scanned);
> 	if (rec == NULL || size != scanned) {
> 		error("Invalid service record!");
> 		goto done;
>@@ -730,7 +730,7 @@ static void remote_svc_identifiers_completed_cb(uint8_t type, uint16_t err,
> 	char **identifiers;
> 	DBusMessage *reply;
> 	GSList *l = NULL;
>-	int scanned, extracted = 0, len = 0, recsize = 0;
>+	int scanned, extracted = 0, len = 0, recsize = 0, bytesleft = size;
> 	uint8_t dtd = 0;
> 
> 	if (!ctxt)
>@@ -762,14 +762,15 @@ static void remote_svc_identifiers_completed_cb(uint8_t type, uint16_t err,
> 		goto failed;
> 	}
> 
>-	scanned = sdp_extract_seqtype(rsp, &dtd, &len);
>+	scanned = sdp_extract_seqtype_safe(rsp, bytesleft, &dtd, &len);
> 	rsp += scanned;
>-	for (; extracted < len; rsp += recsize, extracted += recsize) {
>+	bytesleft -= scanned;
>+	for (; extracted < len; rsp += recsize, extracted += recsize, bytesleft -= recsize) {
> 		sdp_record_t *rec;
> 		sdp_data_t *d;
> 
> 		recsize = 0;
>-		rec = sdp_extract_pdu(rsp, &recsize);
>+		rec = sdp_extract_pdu_safe(rsp, bytesleft, &recsize);
> 		if (!rec)
> 			break;
> 
>diff --git a/sdpd/request.c b/sdpd/request.c
>index 44b65f6..9020dd3 100644
>--- a/sdpd/request.c
>+++ b/sdpd/request.c
>@@ -67,8 +67,9 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
> 	uint8_t dataType;
> 	int status = 0;
> 	const uint8_t *p;
>+	int bufsize;
> 
>-	scanned = sdp_extract_seqtype(buf, &seqType, &data_size);
>+	scanned = sdp_extract_seqtype_safe(buf, len, &seqType, &data_size);
> 
> 	debug("Seq type : %d", seqType);
> 	if (!scanned || (seqType != SDP_SEQ8 && seqType != SDP_SEQ16)) {
>@@ -76,6 +77,7 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
> 		return -1;
> 	}
> 	p = buf + scanned;
>+	bufsize = len - scanned;
> 
> 	debug("Data size : %d", data_size);
> 
>@@ -83,6 +85,11 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
> 		char *pElem = NULL;
> 		int localSeqLength = 0;
> 
>+		if (bufsize < sizeof(uint8_t)) {
>+			debug("->Unexpected end of buffer");
>+			return -1;
>+		}
>+
> 		dataType = *(uint8_t *)p;
> 		debug("Data type: 0x%02x", dataType);
> 
>@@ -100,27 +107,42 @@ static int extract_des(uint8_t *buf, int len, sdp_list_t **svcReqSeq, uint8_t *p
> 		case SDP_UINT16:
> 			p += sizeof(uint8_t);
> 			seqlen += sizeof(uint8_t);
>+			bufsize -= sizeof(uint8_t);
>+			if (bufsize < 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);
> 			seqlen += sizeof(uint16_t);
>+			bufsize -= sizeof(uint16_t);
> 			break;
> 		case SDP_UINT32:
> 			p += sizeof(uint8_t);
> 			seqlen += sizeof(uint8_t);
>+			bufsize -= sizeof(uint8_t);
>+			if (bufsize < (int)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);
>+			bufsize -= sizeof(uint32_t);
> 			break;
> 		case SDP_UUID16:
> 		case SDP_UUID32:
> 		case SDP_UUID128:
> 			pElem = malloc(sizeof(uuid_t));
>-			status = sdp_uuid_extract(p, (uuid_t *)pElem, &localSeqLength);
>+			status = sdp_uuid_extract_safe(p, bufsize, (uuid_t *) pElem, &localSeqLength);
> 			if (status == 0) {
> 				seqlen += localSeqLength;
> 				p += localSeqLength;
>+				bufsize -= localSeqLength;
> 			}
> 			break;
> 		default:
>diff --git a/sdpd/service.c b/sdpd/service.c
>index 57a272d..a1072cd 100644
>--- a/sdpd/service.c
>+++ b/sdpd/service.c
>@@ -412,7 +412,7 @@ int remove_record_from_server(uint32_t handle)
> }
> 
> // 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, int bufsize, uint32_t handleExpected, int *scanned)
> {
> 	int extractStatus = -1, localExtractedLength = 0;
> 	uint8_t dtd;
>@@ -422,13 +422,24 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t h
> 	sdp_data_t *pAttr = NULL;
> 	uint32_t handle = 0xffffffff;
> 
>-	*scanned = sdp_extract_seqtype(p, &dtd, &seqlen);
>+	*scanned = sdp_extract_seqtype_safe(p, bufsize, &dtd, &seqlen);
> 	p += *scanned;
>+	bufsize -= *scanned;
>+
>+	if (bufsize < sizeof(uint8_t) + sizeof(uint8_t)) {
>+		debug("Unexpected end of packet");
>+		return NULL;
>+	}
>+
> 	lookAheadAttrId = ntohs(bt_get_unaligned((uint16_t *) (p + sizeof(uint8_t))));
> 
> 	debug("Look ahead attr id : %d", lookAheadAttrId);
> 
> 	if (lookAheadAttrId == SDP_ATTR_RECORD_HANDLE) {
>+		if (bufsize < (sizeof(uint8_t) * 2) + sizeof(uint16_t) + sizeof(uint32_t)) {
>+			debug("Unexpected end of packet");
>+			return NULL;
>+		}
> 		handle = ntohl(bt_get_unaligned((uint32_t *) (p +
> 				sizeof(uint8_t) + sizeof(uint16_t) +
> 				sizeof(uint8_t))));
>@@ -456,6 +467,11 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t h
> 		int attrSize = sizeof(uint8_t);
> 		int attrValueLength = 0;
> 
>+		if (bufsize < attrSize + sizeof(uint16_t)) {
>+			debug("Unexpected end of packet: Terminating extraction of attributes");
>+			break;
>+		}
>+
> 		debug("Extract PDU, sequenceLength: %d localExtractedLength: %d", seqlen, localExtractedLength);
> 		dtd = *(uint8_t *) p;
> 
>@@ -464,7 +480,8 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t h
> 		
> 		debug("DTD of attrId : %d Attr id : 0x%x", dtd, attrId);
> 
>-		pAttr = sdp_extract_attr(p + attrSize, &attrValueLength, rec);
>+		pAttr = sdp_extract_attr_safe(p + attrSize, bufsize - attrSize,
>+							&attrValueLength, rec);
> 
> 		debug("Attr id : 0x%x attrValueLength : %d", attrId, attrValueLength);
> 
>@@ -475,6 +492,7 @@ static sdp_record_t *extract_pdu_server(bdaddr_t *device, uint8_t *p, uint32_t h
> 		}
> 		localExtractedLength += attrSize;
> 		p += attrSize;
>+		bufsize -= attrSize;
> 		sdp_attr_replace(rec, attrId, pAttr);
> 		extractStatus = 0;
> 		debug("Extract PDU, seqLength: %d localExtractedLength: %d",
>@@ -499,16 +517,18 @@ int service_register_req(sdp_req_t *req, sdp_buf_t *rsp)
> 	int scanned = 0;
> 	sdp_data_t *handle;
> 	uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
>+	int bufsize = req->len - sizeof(sdp_pdu_hdr_t);
> 	sdp_record_t *rec;
> 
> 	req->flags = *p++;
> 	if (req->flags & SDP_DEVICE_RECORD) {
> 		bacpy(&req->device, (bdaddr_t *) p);
> 		p += sizeof(bdaddr_t);
>+		bufsize -= 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, bufsize, 0xffffffff, &scanned);
> 	if (!rec)
> 		goto invalid;
> 
>@@ -566,18 +586,20 @@ int service_update_req(sdp_req_t *req, sdp_buf_t *rsp)
> 	sdp_record_t *orec;
> 	int status = 0, scanned = 0;
> 	uint8_t *p = req->buf + sizeof(sdp_pdu_hdr_t);
>+	int bufsize = 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);
>+	bufsize -= 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, bufsize, handle, &scanned);
> 		if (nrec && handle == nrec->handle) {
> 			update_db_timestamp();
> 			update_svclass_list();

Actions: View | Diff

Attachments on bug 452715: 310163 | 310164 | 310247 | 310250 | 310333 | 310417