* [PATCH BlueZ 1/4] monitor: Decode error response
@ 2018-11-01 14:17 Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 2/4] sdp: Fix not checking if cstate length Luiz Augusto von Dentz
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-01 14:17 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds decoding for the error code in the error response:
> test-sdp: User Data RX
Channel: 0 len 7 [PSM 1 mode 0] {chan 0}
SDP: Error Response (0x01) tid 2 len 2
Error code: Invalid Continuation State (0x0005)
---
monitor/sdp.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/monitor/sdp.c b/monitor/sdp.c
index 36708f426..1df26a0ca 100644
--- a/monitor/sdp.c
+++ b/monitor/sdp.c
@@ -532,6 +532,24 @@ static uint16_t common_rsp(const struct l2cap_frame *frame,
return bytes;
}
+static const char *error_str(uint16_t code)
+{
+ switch (code) {
+ case 0x0001:
+ return "Invalid Version";
+ case 0x0002:
+ return "Invalid Record Handle";
+ case 0x0003:
+ return "Invalid Syntax";
+ case 0x0004:
+ return "Invalid PDU Size";
+ case 0x0005:
+ return "Invalid Continuation State";
+ default:
+ return "Unknown";
+ }
+}
+
static void error_rsp(const struct l2cap_frame *frame, struct tid_data *tid)
{
uint16_t error;
@@ -546,7 +564,7 @@ static void error_rsp(const struct l2cap_frame *frame, struct tid_data *tid)
error = get_be16(frame->data);
- print_field("Error code: 0x%2.2x", error);
+ print_field("Error code: %s (0x%4.4x)", error_str(error), error);
}
static void service_req(const struct l2cap_frame *frame, struct tid_data *tid)
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 2/4] sdp: Fix not checking if cstate length
2018-11-01 14:17 [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
@ 2018-11-01 14:17 ` Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 3/4] sdp: Fix buffer overflow Luiz Augusto von Dentz
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-01 14:17 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
cstate length should be smaller than cached length otherwise the
request shall be considered invalid as the data is not within the
cached buffer.
An independent security researcher, Julian Rauchberger, has reported
this vulnerability to Beyond Security’s SecuriTeam Secure Disclosure
program.
---
src/sdpd-request.c | 74 ++++++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index 318d04467..deaed266f 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -70,9 +70,16 @@ static sdp_buf_t *sdp_get_cached_rsp(sdp_cont_state_t *cstate)
{
sdp_cstate_list_t *p;
- for (p = cstates; p; p = p->next)
- if (p->timestamp == cstate->timestamp)
+ for (p = cstates; p; p = p->next) {
+ /* Check timestamp */
+ if (p->timestamp != cstate->timestamp)
+ continue;
+
+ /* Check if requesting more than available */
+ if (cstate->cStateValue.maxBytesSent < p->buf.data_size)
return &p->buf;
+ }
+
return 0;
}
@@ -624,6 +631,31 @@ static int extract_attrs(sdp_record_t *rec, sdp_list_t *seq, sdp_buf_t *buf)
return 0;
}
+/* Build cstate response */
+static int sdp_cstate_rsp(sdp_cont_state_t *cstate, sdp_buf_t *buf,
+ uint16_t max)
+{
+ /* continuation State exists -> get from cache */
+ sdp_buf_t *cache = sdp_get_cached_rsp(cstate);
+ uint16_t sent;
+
+ if (!cache)
+ return 0;
+
+ sent = MIN(max, cache->data_size - cstate->cStateValue.maxBytesSent);
+ memcpy(buf->data, cache->data + cstate->cStateValue.maxBytesSent, sent);
+ buf->data_size += sent;
+ cstate->cStateValue.maxBytesSent += sent;
+
+ SDPDBG("Response size : %d sending now : %d bytes sent so far : %d",
+ cache->data_size, sent, cstate->cStateValue.maxBytesSent);
+
+ if (cstate->cStateValue.maxBytesSent == cache->data_size)
+ return sdp_set_cstate_pdu(buf, NULL);
+
+ return sdp_set_cstate_pdu(buf, cstate);
+}
+
/*
* A request for the attributes of a service record.
* First check if the service record (specified by
@@ -633,7 +665,6 @@ static int extract_attrs(sdp_record_t *rec, sdp_list_t *seq, sdp_buf_t *buf)
static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
{
sdp_cont_state_t *cstate = NULL;
- uint8_t *pResponse = NULL;
short cstate_size = 0;
sdp_list_t *seq = NULL;
uint8_t dtd = 0;
@@ -719,24 +750,8 @@ static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
buf->buf_size -= sizeof(uint16_t);
if (cstate) {
- sdp_buf_t *pCache = sdp_get_cached_rsp(cstate);
-
- SDPDBG("Obtained cached rsp : %p", pCache);
-
- if (pCache) {
- short sent = MIN(max_rsp_size, pCache->data_size - cstate->cStateValue.maxBytesSent);
- pResponse = pCache->data;
- memcpy(buf->data, pResponse + cstate->cStateValue.maxBytesSent, sent);
- buf->data_size += sent;
- cstate->cStateValue.maxBytesSent += sent;
-
- SDPDBG("Response size : %d sending now : %d bytes sent so far : %d",
- pCache->data_size, sent, cstate->cStateValue.maxBytesSent);
- if (cstate->cStateValue.maxBytesSent == pCache->data_size)
- cstate_size = sdp_set_cstate_pdu(buf, NULL);
- else
- cstate_size = sdp_set_cstate_pdu(buf, cstate);
- } else {
+ cstate_size = sdp_cstate_rsp(cstate, buf, max_rsp_size);
+ if (!cstate_size) {
status = SDP_INVALID_CSTATE;
error("NULL cache buffer and non-NULL continuation state");
}
@@ -786,7 +801,7 @@ done:
static int service_search_attr_req(sdp_req_t *req, sdp_buf_t *buf)
{
int status = 0, plen, totscanned;
- uint8_t *pdata, *pResponse = NULL;
+ uint8_t *pdata;
unsigned int max;
int scanned, rsp_count = 0;
sdp_list_t *pattern = NULL, *seq = NULL, *svcList;
@@ -915,19 +930,8 @@ static int service_search_attr_req(sdp_req_t *req, sdp_buf_t *buf)
} else
cstate_size = sdp_set_cstate_pdu(buf, NULL);
} else {
- /* continuation State exists -> get from cache */
- sdp_buf_t *pCache = sdp_get_cached_rsp(cstate);
- if (pCache && cstate->cStateValue.maxBytesSent < pCache->data_size) {
- uint16_t sent = MIN(max, pCache->data_size - cstate->cStateValue.maxBytesSent);
- pResponse = pCache->data;
- memcpy(buf->data, pResponse + cstate->cStateValue.maxBytesSent, sent);
- buf->data_size += sent;
- cstate->cStateValue.maxBytesSent += sent;
- if (cstate->cStateValue.maxBytesSent == pCache->data_size)
- cstate_size = sdp_set_cstate_pdu(buf, NULL);
- else
- cstate_size = sdp_set_cstate_pdu(buf, cstate);
- } else {
+ cstate_size = sdp_cstate_rsp(cstate, buf, max);
+ if (!cstate_size) {
status = SDP_INVALID_CSTATE;
SDPDBG("Non-null continuation state, but null cache buffer");
}
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 3/4] sdp: Fix buffer overflow
2018-11-01 14:17 [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 2/4] sdp: Fix not checking if cstate length Luiz Augusto von Dentz
@ 2018-11-01 14:17 ` Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 4/4] unit/test-sdp: Add robustness test for continuation state Luiz Augusto von Dentz
2018-11-02 11:12 ` [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-01 14:17 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
sdp_append_buf shall check if there is enough space to store the data
before copying it.
An independent security researcher, Julian Rauchberger, has reported
this vulnerability to Beyond Security’s SecuriTeam Secure Disclosure
program.
---
lib/sdp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index eb408a948..84311eda1 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2834,6 +2834,12 @@ void sdp_append_to_buf(sdp_buf_t *dst, uint8_t *data, uint32_t len)
SDPDBG("Append src size: %d", len);
SDPDBG("Append dst size: %d", dst->data_size);
SDPDBG("Dst buffer size: %d", dst->buf_size);
+
+ if (dst->data_size + len > dst->buf_size) {
+ SDPERR("Cannot append");
+ return;
+ }
+
if (dst->data_size == 0 && dtd == 0) {
/* create initial sequence */
*p = SDP_SEQ8;
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 4/4] unit/test-sdp: Add robustness test for continuation state
2018-11-01 14:17 [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 2/4] sdp: Fix not checking if cstate length Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 3/4] sdp: Fix buffer overflow Luiz Augusto von Dentz
@ 2018-11-01 14:17 ` Luiz Augusto von Dentz
2018-11-02 11:12 ` [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-01 14:17 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds a test for invalid continuation state:
/TP/SERVER/SA/ROB/BI-01-C - init
/TP/SERVER/SA/ROB/BI-01-C - setup
/TP/SERVER/SA/ROB/BI-01-C - setup complete
/TP/SERVER/SA/ROB/BI-01-C - run
test-sdp: < 02 00 01 00 16 35 11 1c 00 00 01 00 00 00 10 00 .....5..........
test-sdp: 80 00 00 80 5f 9b 34 fb 00 01 00 ...._.4....
bluetoothd[26193]: process_request: Got a svc srch req
bluetoothd[26193]: extract_des: Seq type : 53
bluetoothd[26193]: extract_des: Data size : 17
bluetoothd[26193]: extract_des: Data type: 0x1c
bluetoothd[26193]: extract_des: No of elements : 1
bluetoothd[26193]: service_search_req: Expected count: 1
bluetoothd[26193]: service_search_req: Bytes scanned : 19
bluetoothd[26193]: sdp_cstate_get: Continuation State size : 0
bluetoothd[26193]: service_search_req: Checking svcRec : 0x0
bluetoothd[26193]: service_search_req: Checking svcRec : 0x1
bluetoothd[26193]: service_search_req: Checking svcRec : 0x10000
bluetoothd[26193]: service_search_req: Match count: 1
bluetoothd[26193]: process_request: Sending rsp. status 0
bluetoothd[26193]: process_request: Bytes Sent : 14
test-sdp: > 03 00 01 00 09 00 01 00 01 00 01 00 00 00 ..............
test-sdp: < 04 00 01 00 0f 00 01 00 00 00 07 35 06 09 00 00 ...........5....
test-sdp: 09 00 01 00 ....
bluetoothd[26193]: process_request: Got a svc attr req
bluetoothd[26193]: extract_des: Seq type : 53
bluetoothd[26193]: extract_des: Data size : 6
bluetoothd[26193]: extract_des: Data type: 0x09
bluetoothd[26193]: extract_des: No of elements : 1
bluetoothd[26193]: extract_des: Data type: 0x09
bluetoothd[26193]: extract_des: No of elements : 2
bluetoothd[26193]: sdp_cstate_get: Continuation State size : 0
bluetoothd[26193]: service_attr_req: SvcRecHandle : 0x10000
bluetoothd[26193]: service_attr_req: max_rsp_size : 7
bluetoothd[26193]: extract_attrs: Entries in attr seq : 2
bluetoothd[26193]: extract_attrs: AttrDataType : 9
bluetoothd[26193]: extract_attrs: AttrDataType : 9
bluetoothd[26193]: service_attr_req: Creating continuation state of size : 18
bluetoothd[26193]: sdp_set_cstate_pdu: Non null sdp_cstate_t id : 0x5bdb0511
bluetoothd[26193]: process_request: Sending rsp. status 0
bluetoothd[26193]: process_request: Bytes Sent : 23
test-sdp: > 05 00 01 00 12 00 07 35 10 09 00 00 0a 00 08 11 .......5........
test-sdp: 05 db 5b 07 00 00 00 ..[....
test-sdp: < 04 00 02 00 17 00 01 00 00 00 07 35 06 09 00 00 ...........5....
test-sdp: 09 00 01 08 11 05 db 5b ff ff 00 00 .......[....
bluetoothd[26193]: process_request: Got a svc attr req
bluetoothd[26193]: extract_des: Seq type : 53
bluetoothd[26193]: extract_des: Data size : 6
bluetoothd[26193]: extract_des: Data type: 0x09
bluetoothd[26193]: extract_des: No of elements : 1
bluetoothd[26193]: extract_des: Data type: 0x09
bluetoothd[26193]: extract_des: No of elements : 2
bluetoothd[26193]: sdp_cstate_get: Continuation State size : 8
bluetoothd[26193]: sdp_cstate_get: Cstate TS : 0x5bdb0511
bluetoothd[26193]: sdp_cstate_get: Bytes sent : 65535
bluetoothd[26193]: service_attr_req: SvcRecHandle : 0x10000
bluetoothd[26193]: service_attr_req: max_rsp_size : 7
bluetoothd[26193]: NULL cache buffer and non-NULL continuation state
bluetoothd[26193]: process_request: Sending rsp. status 5
bluetoothd[26193]: process_request: Bytes Sent : 7
test-sdp: > 01 00 02 00 02 00 05 .......
/TP/SERVER/SA/ROB/BI-01-C - test passed
/TP/SERVER/SA/ROB/BI-01-C - teardown
/TP/SERVER/SA/ROB/BI-01-C - teardown complete
/TP/SERVER/SA/ROB/BI-01-C - done
---
unit/test-sdp.c | 34 +++++++++++++++++++++++++++++++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index b67a55189..5a50cbbf1 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -45,7 +45,7 @@ struct sdp_pdu {
bool valid;
const void *raw_data;
size_t raw_size;
- uint8_t cont_len;
+ uint16_t cont_len;
};
struct test_data {
@@ -86,6 +86,7 @@ struct test_data {
#define define_sa(name, args...) define_test("/TP/SERVER/SA/" name, 48, args)
#define define_ssa(name, args...) define_test("/TP/SERVER/SSA/" name, 48, args)
#define define_brw(name, args...) define_test("/TP/SERVER/BRW/" name, 672, args)
+#define define_rob(name, args...) define_test("/TP/SERVER/ROB/" name, 48, args)
/* SDP Data Element (DE) tests */
struct test_data_de {
@@ -202,9 +203,13 @@ static gboolean send_pdu(gpointer user_data)
memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
- if (context->cont_size > 0)
+ if (context->cont_size > 0) {
memcpy(buf + req_pdu->raw_size, context->cont_data,
- context->cont_size);
+ context->cont_size);
+ if (context->cont_size != req_pdu->cont_len)
+ put_be16(req_pdu->cont_len,
+ buf + req_pdu->raw_size + 4);
+ }
len = write(context->fd, buf, pdu_len);
@@ -2803,5 +2808,28 @@ int main(int argc, char *argv[])
0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00)));
+ /*
+ * Service Attribute Request
+ *
+ * Verify the correct behaviour of the IUT when searching
+ * for existing Attribute, using invalid continuation state.
+ */
+ define_rob("BI-01-C",
+ raw_pdu(0x02, 0x00, 0x01, 0x00, 0x16, 0x35, 0x11, 0x1c,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb,
+ 0x00, 0x01, 0x00),
+ raw_pdu(0x03, 0x00, 0x01, 0x00, 0x09, 0x00, 0x01, 0x00,
+ 0x01, 0x00, 0x01, 0x00, 0x00, 0x00),
+ raw_pdu(0x04, 0x00, 0x01, 0x00, 0x0f, 0x00, 0x01, 0x00,
+ 0x00, 0x00, 0x07, 0x35, 0x06, 0x09, 0x00, 0x00,
+ 0x09, 0x00, 0x01, 0x00),
+ raw_pdu_cont(8, 0x05, 0x00, 0x01, 0x00, 0x12, 0x00, 0x07, 0x35,
+ 0x10, 0x09, 0x00, 0x00, 0x0a, 0x00, 0x08),
+ raw_pdu_cont(0xffff, 0x04, 0x00, 0x02, 0x00, 0x17, 0x00, 0x01,
+ 0x00, 0x00, 0x00, 0x07, 0x35, 0x06, 0x09, 0x00,
+ 0x00, 0x09, 0x00, 0x01, 0x08),
+ raw_pdu(0x01, 0x00, 0x02, 0x00, 0x02, 0x00, 0x05));
+
return tester_run();
}
--
2.17.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH BlueZ 1/4] monitor: Decode error response
2018-11-01 14:17 [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
` (2 preceding siblings ...)
2018-11-01 14:17 ` [PATCH BlueZ 4/4] unit/test-sdp: Add robustness test for continuation state Luiz Augusto von Dentz
@ 2018-11-02 11:12 ` Luiz Augusto von Dentz
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-02 11:12 UTC (permalink / raw)
To: linux-bluetooth
Hi,
On Thu, Nov 1, 2018 at 4:17 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This adds decoding for the error code in the error response:
>
> > test-sdp: User Data RX
> Channel: 0 len 7 [PSM 1 mode 0] {chan 0}
> SDP: Error Response (0x01) tid 2 len 2
> Error code: Invalid Continuation State (0x0005)
> ---
> monitor/sdp.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/monitor/sdp.c b/monitor/sdp.c
> index 36708f426..1df26a0ca 100644
> --- a/monitor/sdp.c
> +++ b/monitor/sdp.c
> @@ -532,6 +532,24 @@ static uint16_t common_rsp(const struct l2cap_frame *frame,
> return bytes;
> }
>
> +static const char *error_str(uint16_t code)
> +{
> + switch (code) {
> + case 0x0001:
> + return "Invalid Version";
> + case 0x0002:
> + return "Invalid Record Handle";
> + case 0x0003:
> + return "Invalid Syntax";
> + case 0x0004:
> + return "Invalid PDU Size";
> + case 0x0005:
> + return "Invalid Continuation State";
> + default:
> + return "Unknown";
> + }
> +}
> +
> static void error_rsp(const struct l2cap_frame *frame, struct tid_data *tid)
> {
> uint16_t error;
> @@ -546,7 +564,7 @@ static void error_rsp(const struct l2cap_frame *frame, struct tid_data *tid)
>
> error = get_be16(frame->data);
>
> - print_field("Error code: 0x%2.2x", error);
> + print_field("Error code: %s (0x%4.4x)", error_str(error), error);
> }
>
> static void service_req(const struct l2cap_frame *frame, struct tid_data *tid)
> --
> 2.17.2
Applied.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-02 11:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-01 14:17 [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 2/4] sdp: Fix not checking if cstate length Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 3/4] sdp: Fix buffer overflow Luiz Augusto von Dentz
2018-11-01 14:17 ` [PATCH BlueZ 4/4] unit/test-sdp: Add robustness test for continuation state Luiz Augusto von Dentz
2018-11-02 11:12 ` [PATCH BlueZ 1/4] monitor: Decode error response Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.