[PATCH] Fixes buffer overflow when storing too large SDP pdus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



sdp_gen_pdu doesn't check if the given buffer is big enough to contain
the requested data. When storing the SDP records returned by several
Nokia phones, the 512 byte array allocated in the stack by sdp_append_to_pdu 
gets overflown and causes a segmentation fault. 

With this patch, sdp_gen_pdu returns -1 if the buffer is too small and
all the invokers of sdp_gen_pdu will check for errors.

Fixes: https://bugs.launchpad.net/ubuntu/+source/bluez/+bug/332119

PD: This fix is a bit cleaner that previous ones
diff --git a/lib/sdp.c b/lib/sdp.c
index 2581a1d..39d408a 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -661,34 +661,35 @@ void sdp_set_seq_len(uint8_t *ptr, uint32_t length)
 
 static int sdp_set_data_type(sdp_buf_t *buf, uint8_t dtd)
 {
-	int orig = buf->data_size;
-	uint8_t *p = buf->data + buf->data_size;
-
-	*p++ = dtd;
-	buf->data_size += sizeof(uint8_t);
+	size_t size = 1;
 
 	switch (dtd) {
 	case SDP_SEQ8:
 	case SDP_TEXT_STR8:
 	case SDP_URL_STR8:
 	case SDP_ALT8:
-		buf->data_size += sizeof(uint8_t);
+		size += sizeof(uint8_t);
 		break;
 	case SDP_SEQ16:
 	case SDP_TEXT_STR16:
 	case SDP_URL_STR16:
 	case SDP_ALT16:
-		buf->data_size += sizeof(uint16_t);
+		size += sizeof(uint16_t);
 		break;
 	case SDP_SEQ32:
 	case SDP_TEXT_STR32:
 	case SDP_URL_STR32:
 	case SDP_ALT32:
-		buf->data_size += sizeof(uint32_t);
+		size += sizeof(uint32_t);
 		break;
 	}
 
-	return buf->data_size - orig;
+	if (buf->data_size + size > buf->buf_size)
+		return -1;
+
+	buf->data[buf->data_size] = dtd;
+	buf->data_size += size;
+	return size;
 }
 
 void sdp_set_attrid(sdp_buf_t *buf, uint16_t attr)
@@ -709,7 +710,12 @@ static int get_data_size(sdp_buf_t *buf, sdp_data_t *sdpdata)
 	int n = 0;
 
 	for (d = sdpdata->val.dataseq; d; d = d->next)
-		n += sdp_gen_pdu(buf, d);
+	{
+		int res = sdp_gen_pdu(buf, d);
+		if (res < 0)
+			return -1;
+		n += res;
+	}
 
 	return n;
 }
@@ -726,6 +732,8 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 	uint8_t *seqp = buf->data + buf->data_size;
 
 	pdu_size = sdp_set_data_type(buf, dtd);
+	if (pdu_size < 0)
+		return -1;
 
 	switch (dtd) {
 	case SDP_DATA_NIL:
@@ -794,6 +802,8 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 	case SDP_SEQ32:
 		is_seq = 1;
 		data_size = get_data_size(buf, d);
+		if (data_size < 0)
+			return -1;
 		sdp_set_seq_len(seqp, data_size);
 		break;
 	case SDP_ALT8:
@@ -801,6 +811,8 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 	case SDP_ALT32:
 		is_alt = 1;
 		data_size = get_data_size(buf, d);
+		if (data_size < 0)
+			return -1;
 		sdp_set_seq_len(seqp, data_size);
 		break;
 	case SDP_UUID16:
@@ -827,6 +839,7 @@ int sdp_gen_pdu(sdp_buf_t *buf, sdp_data_t *d)
 			buf->data_size += data_size;
 		} else if (dtd != SDP_DATA_NIL) {
 			SDPDBG("Gen PDU : Can't copy from invalid source or dest\n");
+			return -1;
 		}
 	}
 
@@ -2651,8 +2664,8 @@ void sdp_append_to_pdu(sdp_buf_t *pdu, sdp_data_t *d)
 	append.data_size = 0;
 
 	sdp_set_attrid(&append, d->attrId);
-	sdp_gen_pdu(&append, d);
-	sdp_append_to_buf(pdu, append.data, append.data_size);
+	if (sdp_gen_pdu(&append, d) >= 0)
+		sdp_append_to_buf(pdu, append.data, append.data_size);
 }
 
 /*
@@ -3173,6 +3186,11 @@ int sdp_service_search_req(sdp_session_t *session, const sdp_list_t *search,
 
 	// add service class IDs for search
 	seqlen = gen_searchseq_pdu(pdata, search);
+	if (seqlen < 0) {
+		errno = EINVAL;
+		status = -1;
+		goto end;
+	}
 
 	SDPDBG("Data seq added : %d\n", seqlen);
 
@@ -3599,6 +3617,10 @@ int sdp_service_search_async(sdp_session_t *session, const sdp_list_t *search, u
 
 	// add service class IDs for search
 	seqlen = gen_searchseq_pdu(pdata, search);
+	if (seqlen < 0) {
+		t->err = EINVAL;
+		goto end;
+	}
 
 	SDPDBG("Data seq added : %d\n", seqlen);
 
@@ -3818,6 +3840,10 @@ int sdp_service_search_attr_async(sdp_session_t *session, const sdp_list_t *sear
 
 	// add service class IDs for search
 	seqlen = gen_searchseq_pdu(pdata, search);
+	if (seqlen < 0) {
+		t->err = EINVAL;
+		goto end;
+	}
 
 	SDPDBG("Data seq added : %d\n", seqlen);
 
@@ -4169,6 +4195,11 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
 
 	// add service class IDs for search
 	seqlen = gen_searchseq_pdu(pdata, search);
+	if (seqlen < 0) {
+		errno = EINVAL;
+		status = -1;
+		goto end;
+	}
 
 	SDPDBG("Data seq added : %d\n", seqlen);
 

[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux