* [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues
@ 2013-02-10 21:26 Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
` (13 more replies)
0 siblings, 14 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Hi,
This series adds various missing DTD validations, specially for SEQ* types. The
lack of these validations allows for a remote device to crash BlueZ due to
invalid memory access.
I also added unit tests for all affected functions. They are in a separate C
file (unit/test-sdp-lib.c), which can contain tests for SDP library API that is
not covered by SDP server tests in unit/test-sdp.c. It also contains various
tests for corner cases that increase code coverage (as measured by gcov/lcov).
The only pending related fixes from my part are some missing NULL pointer
checks when accessing empty sequences. These will take some time to fix as they
affect profile code as well.
Best Regards,
Anderson Lizardo (13):
unit: Add initial SDP library unit tests
lib: Add SDP_IS_ALT() macro
lib: Reuse identical code in sdp_get_{add,}_access_protos()
lib: Cleanup coding style in sdp_get_proto_descs()
lib: Fix missing DTD validation while accessing SDP data elements
unit: Add tests for sdp_get_lang_attr()
lib: Add missing DTD validation in sdp_record_print()
lib: Validate DTDs when parsing LanguageBaseAttributeIDList
lib: Validate DTDs when parsing BluetoothProfileDescriptorList
lib: Add comment to BluetoothProfileDescriptorList parsing workaround
lib: Validate DTDs when parsing VersionNumberList
unit: Add tests for sdp_get_profile_descs()
unit: Add tests for sdp_get_server_ver()
.gitignore | 1 +
Makefile.am | 5 +-
lib/sdp.c | 164 +++++++++++++-----
lib/sdp.h | 1 +
unit/test-sdp-lib.c | 470 +++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 594 insertions(+), 47 deletions(-)
create mode 100644 unit/test-sdp-lib.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
` (12 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
These tests will cover all public SDP library API not covered yet by the
tests in test-sdp.c (which focus on the SDP server).
---
.gitignore | 1 +
Makefile.am | 5 +-
unit/test-sdp-lib.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 168 insertions(+), 1 deletion(-)
create mode 100644 unit/test-sdp-lib.c
diff --git a/.gitignore b/.gitignore
index b818659..55d6011 100644
--- a/.gitignore
+++ b/.gitignore
@@ -65,6 +65,7 @@ unit/test-uuid
unit/test-textfile
unit/test-gdbus-client
unit/test-sdp
+unit/test-sdp-lib
unit/test-mgmt
tools/mgmt-tester
tools/gap-tester
diff --git a/Makefile.am b/Makefile.am
index 5ebf5e6..fec73ea 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -234,7 +234,7 @@ unit_test_mgmt_SOURCES = unit/test-mgmt.c \
src/shared/mgmt.h src/shared/mgmt.c
unit_test_mgmt_LDADD = @GLIB_LIBS@
-unit_tests += unit/test-sdp
+unit_tests += unit/test-sdp unit/test-sdp-lib
unit_test_sdp_SOURCES = unit/test-sdp.c \
src/shared/util.h src/shared/util.c \
@@ -242,6 +242,9 @@ unit_test_sdp_SOURCES = unit/test-sdp.c \
src/sdpd-service.c src/sdpd-request.c
unit_test_sdp_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
+unit_test_sdp_lib_SOURCES = unit/test-sdp-lib.c
+unit_test_sdp_lib_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
+
unit_tests += unit/test-gdbus-client
unit_test_gdbus_client_SOURCES = $(gdbus_sources) unit/test-gdbus-client.c
diff --git a/unit/test-sdp-lib.c b/unit/test-sdp-lib.c
new file mode 100644
index 0000000..f1f7c0e
--- /dev/null
+++ b/unit/test-sdp-lib.c
@@ -0,0 +1,163 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
+ * Copyright (C) 2013 Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <glib.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+
+static void test_access_protos_valid(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *aproto, *apseq, *proto[2];
+ const uint8_t u8 = 1;
+ uuid_t l2cap, rfcomm;
+ sdp_data_t *channel;
+ int err;
+
+ rec = sdp_record_alloc();
+ sdp_uuid16_create(&l2cap, L2CAP_UUID);
+ proto[0] = sdp_list_append(NULL, &l2cap);
+ apseq = sdp_list_append(NULL, proto[0]);
+
+ sdp_uuid16_create(&rfcomm, RFCOMM_UUID);
+ proto[1] = sdp_list_append(NULL, &rfcomm);
+ channel = sdp_data_alloc(SDP_UINT8, &u8);
+ proto[1] = sdp_list_append(proto[1], channel);
+ apseq = sdp_list_append(apseq, proto[1]);
+
+ aproto = sdp_list_append(NULL, apseq);
+ sdp_set_access_protos(rec, aproto);
+ sdp_set_add_access_protos(rec, aproto);
+ sdp_data_free(channel);
+ sdp_list_free(proto[0], NULL);
+ sdp_list_free(proto[1], NULL);
+ sdp_list_free(apseq, NULL);
+ sdp_list_free(aproto, NULL);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == 0);
+ sdp_list_foreach(aproto, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(aproto, NULL);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == 0);
+ sdp_list_foreach(aproto, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(aproto, NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_access_protos_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == ENODATA);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_access_protos_invalid_dtd1(void)
+{
+ const uint32_t u32 = 0xdeadbeeb;
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ sdp_data_t *data;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_PROTO_DESC_LIST, data);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_ADD_PROTO_DESC_LIST, data);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_access_protos_invalid_dtd2(void)
+{
+ uint8_t dtd = SDP_UINT8, u8 = 0xff;
+ void *dtds = &dtd, *values = &u8;
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ sdp_data_t *data;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ data = sdp_seq_alloc(&dtds, &values, 1);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_PROTO_DESC_LIST, data);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ data = sdp_seq_alloc(&dtds, &values, 1);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_ADD_PROTO_DESC_LIST, data);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_func("/sdp/lib/access_protos/valid",
+ test_access_protos_valid);
+ g_test_add_func("/sdp/lib/access_protos/nodata",
+ test_access_protos_nodata);
+ g_test_add_func("/sdp/lib/access_protos/invalid_dtd1",
+ test_access_protos_invalid_dtd1);
+ g_test_add_func("/sdp/lib/access_protos/invalid_dtd2",
+ test_access_protos_invalid_dtd2);
+
+ return g_test_run();
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
` (11 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
This macro will simplify internal SDP library code, and is similar to
the existing SDP_IS_SEQ() macro.
---
lib/sdp.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/sdp.h b/lib/sdp.h
index a81e857..baae15f 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -440,6 +440,7 @@ typedef struct {
#define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || \
(x) == SDP_UUID128)
+#define SDP_IS_ALT(x) ((x) == SDP_ALT8 || (x) == SDP_ALT16 || (x) == SDP_ALT32)
#define SDP_IS_SEQ(x) ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
#define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
(x) == SDP_TEXT_STR32)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos()
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
` (10 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
sdp_get_access_protos() and sdp_get_add_access_protos() do almost
exactly the same thing, except for an additional statement for the
latter.
---
lib/sdp.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index b87f392..c4afed7 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1860,18 +1860,22 @@ sdp_data_t *sdp_get_proto_desc(sdp_list_t *list, int proto)
return NULL;
}
-int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
+ sdp_list_t **pap)
{
sdp_data_t *pdlist, *curr;
sdp_list_t *ap = 0;
- pdlist = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
+ pdlist = sdp_data_get(rec, attr_id);
if (pdlist == NULL) {
errno = ENODATA;
return -1;
}
SDPDBG("AP type : 0%x\n", pdlist->dtd);
+ if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
+ pdlist = pdlist->val.dataseq;
+
for (; pdlist; pdlist = pdlist->next) {
sdp_list_t *pds = 0;
for (curr = pdlist->val.dataseq; curr; curr = curr->next)
@@ -1882,28 +1886,14 @@ int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
return 0;
}
-int sdp_get_add_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
{
- sdp_data_t *pdlist, *curr;
- sdp_list_t *ap = 0;
-
- pdlist = sdp_data_get(rec, SDP_ATTR_ADD_PROTO_DESC_LIST);
- if (pdlist == NULL) {
- errno = ENODATA;
- return -1;
- }
- SDPDBG("AP type : 0%x\n", pdlist->dtd);
-
- pdlist = pdlist->val.dataseq;
+ return sdp_get_proto_descs(SDP_ATTR_PROTO_DESC_LIST, rec, pap);
+}
- for (; pdlist; pdlist = pdlist->next) {
- sdp_list_t *pds = 0;
- for (curr = pdlist->val.dataseq; curr; curr = curr->next)
- pds = sdp_list_append(pds, curr->val.dataseq);
- ap = sdp_list_append(ap, pds);
- }
- *pap = ap;
- return 0;
+int sdp_get_add_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+{
+ return sdp_get_proto_descs(SDP_ATTR_ADD_PROTO_DESC_LIST, rec, pap);
}
int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs()
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (2 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
` (9 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
lib/sdp.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index c4afed7..aa67f2a 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1864,25 +1864,28 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
sdp_list_t **pap)
{
sdp_data_t *pdlist, *curr;
- sdp_list_t *ap = 0;
+ sdp_list_t *ap = NULL;
pdlist = sdp_data_get(rec, attr_id);
if (pdlist == NULL) {
errno = ENODATA;
return -1;
}
- SDPDBG("AP type : 0%x\n", pdlist->dtd);
+
+ SDPDBG("Attribute value type: 0x%02x\n", pdlist->dtd);
if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
pdlist = pdlist->val.dataseq;
for (; pdlist; pdlist = pdlist->next) {
- sdp_list_t *pds = 0;
+ sdp_list_t *pds = NULL;
for (curr = pdlist->val.dataseq; curr; curr = curr->next)
pds = sdp_list_append(pds, curr->val.dataseq);
ap = sdp_list_append(ap, pds);
}
+
*pap = ap;
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (3 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
` (8 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
It is necessary to validate the sdp_data_t "dtd" field before accessing
the "val" union members, specially when handling SDP_SEQ*, SDP_ALT* and
SDP_STR* elements, otherwise remote devices can trigger memory
corruption by passing invalid data elements where others are expected.
---
lib/sdp.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index aa67f2a..f65138c 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1874,19 +1874,41 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
SDPDBG("Attribute value type: 0x%02x\n", pdlist->dtd);
- if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
+ if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST) {
+ if (!SDP_IS_SEQ(pdlist->dtd)) {
+ errno = EINVAL;
+ return -1;
+ }
pdlist = pdlist->val.dataseq;
+ }
for (; pdlist; pdlist = pdlist->next) {
sdp_list_t *pds = NULL;
- for (curr = pdlist->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(pdlist->dtd) && !SDP_IS_ALT(pdlist->dtd))
+ goto failed;
+
+ for (curr = pdlist->val.dataseq; curr; curr = curr->next) {
+ if (!SDP_IS_SEQ(curr->dtd)) {
+ sdp_list_free(pds, NULL);
+ goto failed;
+ }
pds = sdp_list_append(pds, curr->val.dataseq);
+ }
+
ap = sdp_list_append(ap, pds);
}
*pap = ap;
return 0;
+
+failed:
+ sdp_list_foreach(ap, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(ap, NULL);
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr()
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (4 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
` (7 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
unit/test-sdp-lib.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/unit/test-sdp-lib.c b/unit/test-sdp-lib.c
index f1f7c0e..51e5873 100644
--- a/unit/test-sdp-lib.c
+++ b/unit/test-sdp-lib.c
@@ -146,6 +146,86 @@ static void test_access_protos_invalid_dtd2(void)
sdp_record_free(rec);
}
+static void test_language_base_valid(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+ sdp_add_lang_attr(rec);
+
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == 0);
+
+ sdp_list_free(list, free);
+ sdp_record_free(rec);
+}
+
+static void test_language_base_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_language_base_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds1[] = { &dtd1, &dtd2, &dtd2 };
+ void *values1[] = { &u16, &u32, &u32 };
+ void *dtds2[] = { &dtd1, &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds1[1], &values1[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT16) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT32, UINT32) */
+ data = sdp_seq_alloc(dtds1, values1, 3);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 3);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -159,5 +239,12 @@ int main(int argc, char *argv[])
g_test_add_func("/sdp/lib/access_protos/invalid_dtd2",
test_access_protos_invalid_dtd2);
+ g_test_add_func("/sdp/lib/language_base/valid",
+ test_language_base_valid);
+ g_test_add_func("/sdp/lib/language_base/nodata",
+ test_language_base_nodata);
+ g_test_add_func("/sdp/lib/language_base/invalid_dtd",
+ test_language_base_invalid_dtd);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print()
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (5 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
` (6 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
lib/sdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index f65138c..21dfe1b 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1603,13 +1603,13 @@ static void print_dataseq(sdp_data_t *p)
void sdp_record_print(const sdp_record_t *rec)
{
sdp_data_t *d = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Name: %.*s\n", d->unitSize, d->val.str);
d = sdp_data_get(rec, SDP_ATTR_SVCDESC_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Description: %.*s\n", d->unitSize, d->val.str);
d = sdp_data_get(rec, SDP_ATTR_PROVNAME_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Provider: %.*s\n", d->unitSize, d->val.str);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (6 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
` (5 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Also check if the required number of entries is present.
---
lib/sdp.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 21dfe1b..ba55fc4 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2019,19 +2019,30 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
errno = ENODATA;
return -1;
}
+
+ if (!SDP_IS_SEQ(sdpdata->dtd))
+ goto invalid;
curr_data = sdpdata->val.dataseq;
+
while (curr_data) {
- sdp_data_t *pCode = curr_data;
- sdp_data_t *pEncoding;
- sdp_data_t *pOffset;
+ sdp_data_t *pCode, *pEncoding, *pOffset;
+
+ pCode = curr_data;
+ if (pCode->dtd != SDP_UINT16)
+ goto invalid;
+
+ /* LanguageBaseAttributeIDList entries are always grouped as
+ * triplets */
+ if (!pCode->next || !pCode->next->next)
+ goto invalid;
pEncoding = pCode->next;
- if (!pEncoding)
- break;
+ if (pEncoding->dtd != SDP_UINT16)
+ goto invalid;
pOffset = pEncoding->next;
- if (!pOffset)
- break;
+ if (pOffset->dtd != SDP_UINT16)
+ goto invalid;
lang = malloc(sizeof(sdp_lang_attr_t));
if (!lang) {
@@ -2051,6 +2062,13 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
}
return 0;
+
+invalid:
+ sdp_list_free(*langSeq, free);
+ *langSeq = NULL;
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (7 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
` (4 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
The "seq->val.dataseq != NULL" check is also removed from the for()
statement because it should be done after verifying that the data
element is a sequence (inside the "if (SDP_IS_SEQ(...))" block.)
---
lib/sdp.c | 37 ++++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index ba55fc4..c5bc16e 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2078,11 +2078,15 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
*profDescSeq = NULL;
sdpdata = sdp_data_get(rec, SDP_ATTR_PFILE_DESC_LIST);
- if (!sdpdata || !sdpdata->val.dataseq) {
+ if (sdpdata == NULL) {
errno = ENODATA;
return -1;
}
- for (seq = sdpdata->val.dataseq; seq && seq->val.dataseq; seq = seq->next) {
+
+ if (!SDP_IS_SEQ(sdpdata->dtd) || sdpdata->val.dataseq == NULL)
+ goto invalid;
+
+ for (seq = sdpdata->val.dataseq; seq; seq = seq->next) {
uuid_t *uuid = NULL;
uint16_t version = 0x100;
@@ -2094,13 +2098,21 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
seq = next;
}
} else if (SDP_IS_SEQ(seq->dtd)) {
- sdp_data_t *puuid = seq->val.dataseq;
- sdp_data_t *pVnum = seq->val.dataseq->next;
- if (puuid && pVnum) {
- uuid = &puuid->val.uuid;
- version = pVnum->val.uint16;
- }
- }
+ sdp_data_t *puuid, *pVnum;
+
+ puuid = seq->val.dataseq;
+ if (puuid == NULL || !SDP_IS_UUID(puuid->dtd))
+ goto invalid;
+
+ uuid = &puuid->val.uuid;
+
+ pVnum = puuid->next;
+ if (pVnum == NULL || pVnum->dtd != SDP_UINT16)
+ goto invalid;
+
+ version = pVnum->val.uint16;
+ } else
+ goto invalid;
if (uuid != NULL) {
profDesc = malloc(sizeof(sdp_profile_desc_t));
@@ -2119,6 +2131,13 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
}
}
return 0;
+
+invalid:
+ sdp_list_free(*profDescSeq, free);
+ *profDescSeq = NULL;
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (8 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
` (3 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Commits 0f5a5a9580084a3c4e0644ef5cd75689aeb5ff40 and
46b3a3d2d00bf70bc57ef0c9ad5542a2271e3350 introduced this workaround.
---
lib/sdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index c5bc16e..a8a4010 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2091,6 +2091,11 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
uint16_t version = 0x100;
if (SDP_IS_UUID(seq->dtd)) {
+ /* Mac OS X 10.7.3 and old Samsung phones do not comply
+ * to the SDP specification for
+ * BluetoothProfileDescriptorList. This workaround
+ * allows to properly parse UUID/version from SDP
+ * record published by these systems. */
sdp_data_t *next = seq->next;
uuid = &seq->val.uuid;
if (next && next->dtd == SDP_UINT16) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (9 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
` (2 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
lib/sdp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index a8a4010..bb7dc6b 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2155,9 +2155,24 @@ int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
errno = ENODATA;
return -1;
}
- for (curr = d->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(d->dtd) || d->val.dataseq == NULL)
+ goto invalid;
+
+ for (curr = d->val.dataseq; curr; curr = curr->next) {
+ if (curr->dtd != SDP_UINT16)
+ goto invalid;
*u16 = sdp_list_append(*u16, &curr->val.uint16);
+ }
+
return 0;
+
+invalid:
+ sdp_list_free(*u16, NULL);
+ *u16 = NULL;
+ errno = EINVAL;
+
+ return -1;
}
/* flexible extraction of basic attributes - Jean II */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs()
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (10 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
unit/test-sdp-lib.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 171 insertions(+)
diff --git a/unit/test-sdp-lib.c b/unit/test-sdp-lib.c
index 51e5873..346942f 100644
--- a/unit/test-sdp-lib.c
+++ b/unit/test-sdp-lib.c
@@ -226,6 +226,167 @@ static void test_language_base_invalid_dtd(void)
sdp_record_free(rec);
}
+static void test_profile_desc_valid(void)
+{
+ sdp_profile_desc_t profile;
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ sdp_uuid16_create(&profile.uuid, NAP_PROFILE_ID);
+ profile.version = 0x0100;
+ list = sdp_list_append(NULL, &profile);
+ err = sdp_set_profile_descs(rec, list);
+ sdp_list_free(list, NULL);
+ g_assert(err == 0);
+
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_profile_desc_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_profile_desc_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds[1], *values[1];
+ void *dtds2[] = { &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8()) */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UINT32)) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16)) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16, UINT32)) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_profile_desc_workaround(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT16, dtd3 = SDP_UINT32;
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ void *dtds[] = { &dtd1, &dtd2 };
+ void *values[] = { &u16, &u16 };
+ void *dtds2[] = { &dtd1, &dtd3 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT16) */
+ data = sdp_seq_alloc(dtds, values, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -246,5 +407,15 @@ int main(int argc, char *argv[])
g_test_add_func("/sdp/lib/language_base/invalid_dtd",
test_language_base_invalid_dtd);
+ g_test_add_func("/sdp/lib/profile_desc/valid",
+ test_profile_desc_valid);
+ g_test_add_func("/sdp/lib/profile_desc/nodata",
+ test_profile_desc_nodata);
+ g_test_add_func("/sdp/lib/profile_desc/invalid_dtd",
+ test_profile_desc_invalid_dtd);
+ /* Test for workaround commented on sdp_get_profile_descs() */
+ g_test_add_func("/sdp/lib/profile_desc/workaround",
+ test_profile_desc_workaround);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver()
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (11 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
@ 2013-02-10 21:26 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-10 21:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
unit/test-sdp-lib.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/unit/test-sdp-lib.c b/unit/test-sdp-lib.c
index 346942f..a7480c3 100644
--- a/unit/test-sdp-lib.c
+++ b/unit/test-sdp-lib.c
@@ -387,6 +387,53 @@ static void test_profile_desc_workaround(void)
sdp_record_free(rec);
}
+static void test_server_ver(void)
+{
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ void *dtds1[] = { &dtd1 };
+ void *values1[] = { &u16 };
+ void *dtds2[] = { &dtd2 };
+ void *values2[] = { &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ /* Valid DTD */
+ data = sdp_seq_alloc(dtds1, values1, 1);
+ sdp_attr_add(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == 0 && list != NULL);
+ sdp_list_free(list, NULL);
+
+ /* Invalid: UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8(UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -417,5 +464,7 @@ int main(int argc, char *argv[])
g_test_add_func("/sdp/lib/profile_desc/workaround",
test_profile_desc_workaround);
+ g_test_add_func("/sdp/lib/server_ver", test_server_ver);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
@ 2013-02-15 14:51 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
` (13 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:51 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Hi,
Just noticed that "From:" field in my patches got mangled due to
importing from gmane archives. Will fix ASAP and resend.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (12 preceding siblings ...)
2013-02-10 21:26 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:51 ` Anderson Lizardo
` (14 more replies)
13 siblings, 15 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Hi,
Changes since v1:
* Fix license header to match BlueZ license (GPL v2 or later)
* Rename test source file and SDP tests to account for future addition of other
libbluetooth tests
This series adds various missing DTD validations, specially for SEQ* types. The
lack of these validations allows for a remote device to crash BlueZ due to
invalid memory access.
I also added unit tests for all affected functions. They are in a separate C
file (unit/test-lib.c), which will in future contain tests for other
libbluetooth API functions.
The only pending related fixes from my part are some missing NULL pointer
checks when accessing empty sequences. These will take some time to fix as they
affect profile code as well.
Best Regards,
Anderson Lizardo (13):
unit: Add initial SDP library unit tests
lib: Add SDP_IS_ALT() macro
lib: Reuse identical code in sdp_get_{add,}_access_protos()
lib: Cleanup coding style in sdp_get_proto_descs()
lib: Fix missing DTD validation while accessing SDP data elements
unit: Add tests for sdp_get_lang_attr()
lib: Add missing DTD validation in sdp_record_print()
lib: Validate DTDs when parsing LanguageBaseAttributeIDList
lib: Validate DTDs when parsing BluetoothProfileDescriptorList
lib: Add comment to BluetoothProfileDescriptorList parsing workaround
lib: Validate DTDs when parsing VersionNumberList
unit: Add tests for sdp_get_profile_descs()
unit: Add tests for sdp_get_server_ver()
.gitignore | 1 +
Makefile.am | 5 +
lib/sdp.c | 164 +++++++++++++------
lib/sdp.h | 1 +
unit/test-lib.c | 471 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 596 insertions(+), 46 deletions(-)
create mode 100644 unit/test-lib.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-15 14:51 ` Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
` (12 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
These tests will cover all public SDP library API not covered yet by the
tests in test-sdp.c (which focus on the SDP server).
---
.gitignore | 1 +
Makefile.am | 5 ++
unit/test-lib.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 170 insertions(+)
create mode 100644 unit/test-lib.c
diff --git a/.gitignore b/.gitignore
index b818659..9483b92 100644
--- a/.gitignore
+++ b/.gitignore
@@ -65,6 +65,7 @@ unit/test-uuid
unit/test-textfile
unit/test-gdbus-client
unit/test-sdp
+unit/test-lib
unit/test-mgmt
tools/mgmt-tester
tools/gap-tester
diff --git a/Makefile.am b/Makefile.am
index 5ebf5e6..cbcde3e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -270,6 +270,11 @@ unit_test_gobex_apparam_SOURCES = $(gobex_sources) unit/util.c unit/util.h \
unit/test-gobex-apparam.c
unit_test_gobex_apparam_LDADD = @GLIB_LIBS@
+unit_tests += unit/test-lib
+
+unit_test_lib_SOURCES = unit/test-lib.c
+unit_test_lib_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
+
noinst_PROGRAMS += $(unit_tests)
TESTS = $(unit_tests)
diff --git a/unit/test-lib.c b/unit/test-lib.c
new file mode 100644
index 0000000..0be6d80
--- /dev/null
+++ b/unit/test-lib.c
@@ -0,0 +1,164 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
+ * Copyright (C) 2013 Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <glib.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+
+static void test_sdp_get_access_protos_valid(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *aproto, *apseq, *proto[2];
+ const uint8_t u8 = 1;
+ uuid_t l2cap, rfcomm;
+ sdp_data_t *channel;
+ int err;
+
+ rec = sdp_record_alloc();
+ sdp_uuid16_create(&l2cap, L2CAP_UUID);
+ proto[0] = sdp_list_append(NULL, &l2cap);
+ apseq = sdp_list_append(NULL, proto[0]);
+
+ sdp_uuid16_create(&rfcomm, RFCOMM_UUID);
+ proto[1] = sdp_list_append(NULL, &rfcomm);
+ channel = sdp_data_alloc(SDP_UINT8, &u8);
+ proto[1] = sdp_list_append(proto[1], channel);
+ apseq = sdp_list_append(apseq, proto[1]);
+
+ aproto = sdp_list_append(NULL, apseq);
+ sdp_set_access_protos(rec, aproto);
+ sdp_set_add_access_protos(rec, aproto);
+ sdp_data_free(channel);
+ sdp_list_free(proto[0], NULL);
+ sdp_list_free(proto[1], NULL);
+ sdp_list_free(apseq, NULL);
+ sdp_list_free(aproto, NULL);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == 0);
+ sdp_list_foreach(aproto, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(aproto, NULL);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == 0);
+ sdp_list_foreach(aproto, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(aproto, NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_access_protos_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == ENODATA);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_access_protos_invalid_dtd1(void)
+{
+ const uint32_t u32 = 0xdeadbeeb;
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ sdp_data_t *data;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_PROTO_DESC_LIST, data);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_ADD_PROTO_DESC_LIST, data);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_access_protos_invalid_dtd2(void)
+{
+ uint8_t dtd = SDP_UINT8, u8 = 0xff;
+ void *dtds = &dtd, *values = &u8;
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ sdp_data_t *data;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ data = sdp_seq_alloc(&dtds, &values, 1);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_PROTO_DESC_LIST, data);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ data = sdp_seq_alloc(&dtds, &values, 1);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_ADD_PROTO_DESC_LIST, data);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_func("/lib/sdp_get_access_protos/valid",
+ test_sdp_get_access_protos_valid);
+ g_test_add_func("/lib/sdp_get_access_protos/nodata",
+ test_sdp_get_access_protos_nodata);
+ g_test_add_func("/lib/sdp_get_access_protos/invalid_dtd1",
+ test_sdp_get_access_protos_invalid_dtd1);
+ g_test_add_func("/lib/sdp_get_access_protos/invalid_dtd2",
+ test_sdp_get_access_protos_invalid_dtd2);
+
+ return g_test_run();
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-15 14:51 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
` (11 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
This macro will simplify internal SDP library code, and is similar to
the existing SDP_IS_SEQ() macro.
---
lib/sdp.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/sdp.h b/lib/sdp.h
index a81e857..baae15f 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -440,6 +440,7 @@ typedef struct {
#define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || \
(x) == SDP_UUID128)
+#define SDP_IS_ALT(x) ((x) == SDP_ALT8 || (x) == SDP_ALT16 || (x) == SDP_ALT32)
#define SDP_IS_SEQ(x) ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
#define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
(x) == SDP_TEXT_STR32)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos()
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (2 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
` (10 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
sdp_get_access_protos() and sdp_get_add_access_protos() do almost
exactly the same thing, except for an additional statement for the
latter.
---
lib/sdp.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 8cfdde1..57abfd8 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1860,18 +1860,22 @@ sdp_data_t *sdp_get_proto_desc(sdp_list_t *list, int proto)
return NULL;
}
-int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
+ sdp_list_t **pap)
{
sdp_data_t *pdlist, *curr;
sdp_list_t *ap = 0;
- pdlist = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
+ pdlist = sdp_data_get(rec, attr_id);
if (pdlist == NULL) {
errno = ENODATA;
return -1;
}
SDPDBG("AP type : 0%x\n", pdlist->dtd);
+ if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
+ pdlist = pdlist->val.dataseq;
+
for (; pdlist; pdlist = pdlist->next) {
sdp_list_t *pds = 0;
for (curr = pdlist->val.dataseq; curr; curr = curr->next)
@@ -1882,28 +1886,14 @@ int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
return 0;
}
-int sdp_get_add_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
{
- sdp_data_t *pdlist, *curr;
- sdp_list_t *ap = 0;
-
- pdlist = sdp_data_get(rec, SDP_ATTR_ADD_PROTO_DESC_LIST);
- if (pdlist == NULL) {
- errno = ENODATA;
- return -1;
- }
- SDPDBG("AP type : 0%x\n", pdlist->dtd);
-
- pdlist = pdlist->val.dataseq;
+ return sdp_get_proto_descs(SDP_ATTR_PROTO_DESC_LIST, rec, pap);
+}
- for (; pdlist; pdlist = pdlist->next) {
- sdp_list_t *pds = 0;
- for (curr = pdlist->val.dataseq; curr; curr = curr->next)
- pds = sdp_list_append(pds, curr->val.dataseq);
- ap = sdp_list_append(ap, pds);
- }
- *pap = ap;
- return 0;
+int sdp_get_add_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+{
+ return sdp_get_proto_descs(SDP_ATTR_ADD_PROTO_DESC_LIST, rec, pap);
}
int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs()
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (3 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
` (9 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
---
lib/sdp.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 57abfd8..3c45ac8 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1864,25 +1864,28 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
sdp_list_t **pap)
{
sdp_data_t *pdlist, *curr;
- sdp_list_t *ap = 0;
+ sdp_list_t *ap = NULL;
pdlist = sdp_data_get(rec, attr_id);
if (pdlist == NULL) {
errno = ENODATA;
return -1;
}
- SDPDBG("AP type : 0%x\n", pdlist->dtd);
+
+ SDPDBG("Attribute value type: 0x%02x\n", pdlist->dtd);
if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
pdlist = pdlist->val.dataseq;
for (; pdlist; pdlist = pdlist->next) {
- sdp_list_t *pds = 0;
+ sdp_list_t *pds = NULL;
for (curr = pdlist->val.dataseq; curr; curr = curr->next)
pds = sdp_list_append(pds, curr->val.dataseq);
ap = sdp_list_append(ap, pds);
}
+
*pap = ap;
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (4 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
` (8 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
It is necessary to validate the sdp_data_t "dtd" field before accessing
the "val" union members, specially when handling SDP_SEQ*, SDP_ALT* and
SDP_STR* elements, otherwise remote devices can trigger memory
corruption by passing invalid data elements where others are expected.
---
lib/sdp.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 3c45ac8..9c1eeb5 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1874,19 +1874,41 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
SDPDBG("Attribute value type: 0x%02x\n", pdlist->dtd);
- if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
+ if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST) {
+ if (!SDP_IS_SEQ(pdlist->dtd)) {
+ errno = EINVAL;
+ return -1;
+ }
pdlist = pdlist->val.dataseq;
+ }
for (; pdlist; pdlist = pdlist->next) {
sdp_list_t *pds = NULL;
- for (curr = pdlist->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(pdlist->dtd) && !SDP_IS_ALT(pdlist->dtd))
+ goto failed;
+
+ for (curr = pdlist->val.dataseq; curr; curr = curr->next) {
+ if (!SDP_IS_SEQ(curr->dtd)) {
+ sdp_list_free(pds, NULL);
+ goto failed;
+ }
pds = sdp_list_append(pds, curr->val.dataseq);
+ }
+
ap = sdp_list_append(ap, pds);
}
*pap = ap;
return 0;
+
+failed:
+ sdp_list_foreach(ap, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(ap, NULL);
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr()
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (5 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
` (7 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
---
unit/test-lib.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index 0be6d80..76e4363 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -147,6 +147,86 @@ static void test_sdp_get_access_protos_invalid_dtd2(void)
sdp_record_free(rec);
}
+static void test_sdp_get_lang_attr_valid(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+ sdp_add_lang_attr(rec);
+
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == 0);
+
+ sdp_list_free(list, free);
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_lang_attr_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_lang_attr_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds1[] = { &dtd1, &dtd2, &dtd2 };
+ void *values1[] = { &u16, &u32, &u32 };
+ void *dtds2[] = { &dtd1, &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds1[1], &values1[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT16) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT32, UINT32) */
+ data = sdp_seq_alloc(dtds1, values1, 3);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 3);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -160,5 +240,12 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_access_protos/invalid_dtd2",
test_sdp_get_access_protos_invalid_dtd2);
+ g_test_add_func("/lib/sdp_get_lang_attr/valid",
+ test_sdp_get_lang_attr_valid);
+ g_test_add_func("/lib/sdp_get_lang_attr/nodata",
+ test_sdp_get_lang_attr_nodata);
+ g_test_add_func("/lib/sdp_get_lang_attr/invalid_dtd",
+ test_sdp_get_lang_attr_invalid_dtd);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print()
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (6 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
` (6 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
---
lib/sdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 9c1eeb5..782d9bd 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1603,13 +1603,13 @@ static void print_dataseq(sdp_data_t *p)
void sdp_record_print(const sdp_record_t *rec)
{
sdp_data_t *d = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Name: %.*s\n", d->unitSize, d->val.str);
d = sdp_data_get(rec, SDP_ATTR_SVCDESC_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Description: %.*s\n", d->unitSize, d->val.str);
d = sdp_data_get(rec, SDP_ATTR_PROVNAME_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Provider: %.*s\n", d->unitSize, d->val.str);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (7 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
` (5 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
Also check if the required number of entries is present.
---
lib/sdp.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 782d9bd..77dafaa 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2019,19 +2019,30 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
errno = ENODATA;
return -1;
}
+
+ if (!SDP_IS_SEQ(sdpdata->dtd))
+ goto invalid;
curr_data = sdpdata->val.dataseq;
+
while (curr_data) {
- sdp_data_t *pCode = curr_data;
- sdp_data_t *pEncoding;
- sdp_data_t *pOffset;
+ sdp_data_t *pCode, *pEncoding, *pOffset;
+
+ pCode = curr_data;
+ if (pCode->dtd != SDP_UINT16)
+ goto invalid;
+
+ /* LanguageBaseAttributeIDList entries are always grouped as
+ * triplets */
+ if (!pCode->next || !pCode->next->next)
+ goto invalid;
pEncoding = pCode->next;
- if (!pEncoding)
- break;
+ if (pEncoding->dtd != SDP_UINT16)
+ goto invalid;
pOffset = pEncoding->next;
- if (!pOffset)
- break;
+ if (pOffset->dtd != SDP_UINT16)
+ goto invalid;
lang = malloc(sizeof(sdp_lang_attr_t));
if (!lang) {
@@ -2051,6 +2062,13 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
}
return 0;
+
+invalid:
+ sdp_list_free(*langSeq, free);
+ *langSeq = NULL;
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (8 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
` (4 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
The "seq->val.dataseq != NULL" check is also removed from the for()
statement because it should be done after verifying that the data
element is a sequence (inside the "if (SDP_IS_SEQ(...))" block.)
---
lib/sdp.c | 37 ++++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 77dafaa..396567f 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2078,11 +2078,15 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
*profDescSeq = NULL;
sdpdata = sdp_data_get(rec, SDP_ATTR_PFILE_DESC_LIST);
- if (!sdpdata || !sdpdata->val.dataseq) {
+ if (sdpdata == NULL) {
errno = ENODATA;
return -1;
}
- for (seq = sdpdata->val.dataseq; seq && seq->val.dataseq; seq = seq->next) {
+
+ if (!SDP_IS_SEQ(sdpdata->dtd) || sdpdata->val.dataseq == NULL)
+ goto invalid;
+
+ for (seq = sdpdata->val.dataseq; seq; seq = seq->next) {
uuid_t *uuid = NULL;
uint16_t version = 0x100;
@@ -2094,13 +2098,21 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
seq = next;
}
} else if (SDP_IS_SEQ(seq->dtd)) {
- sdp_data_t *puuid = seq->val.dataseq;
- sdp_data_t *pVnum = seq->val.dataseq->next;
- if (puuid && pVnum) {
- uuid = &puuid->val.uuid;
- version = pVnum->val.uint16;
- }
- }
+ sdp_data_t *puuid, *pVnum;
+
+ puuid = seq->val.dataseq;
+ if (puuid == NULL || !SDP_IS_UUID(puuid->dtd))
+ goto invalid;
+
+ uuid = &puuid->val.uuid;
+
+ pVnum = puuid->next;
+ if (pVnum == NULL || pVnum->dtd != SDP_UINT16)
+ goto invalid;
+
+ version = pVnum->val.uint16;
+ } else
+ goto invalid;
if (uuid != NULL) {
profDesc = malloc(sizeof(sdp_profile_desc_t));
@@ -2119,6 +2131,13 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
}
}
return 0;
+
+invalid:
+ sdp_list_free(*profDescSeq, free);
+ *profDescSeq = NULL;
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (9 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
` (3 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
Commits 0f5a5a9580084a3c4e0644ef5cd75689aeb5ff40 and
46b3a3d2d00bf70bc57ef0c9ad5542a2271e3350 introduced this workaround.
---
lib/sdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index 396567f..61598bb 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2091,6 +2091,11 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
uint16_t version = 0x100;
if (SDP_IS_UUID(seq->dtd)) {
+ /* Mac OS X 10.7.3 and old Samsung phones do not comply
+ * to the SDP specification for
+ * BluetoothProfileDescriptorList. This workaround
+ * allows to properly parse UUID/version from SDP
+ * record published by these systems. */
sdp_data_t *next = seq->next;
uuid = &seq->val.uuid;
if (next && next->dtd == SDP_UINT16) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (10 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
` (2 subsequent siblings)
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
---
lib/sdp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 61598bb..6c73818 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2155,9 +2155,24 @@ int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
errno = ENODATA;
return -1;
}
- for (curr = d->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(d->dtd) || d->val.dataseq == NULL)
+ goto invalid;
+
+ for (curr = d->val.dataseq; curr; curr = curr->next) {
+ if (curr->dtd != SDP_UINT16)
+ goto invalid;
*u16 = sdp_list_append(*u16, &curr->val.uint16);
+ }
+
return 0;
+
+invalid:
+ sdp_list_free(*u16, NULL);
+ *u16 = NULL;
+ errno = EINVAL;
+
+ return -1;
}
/* flexible extraction of basic attributes - Jean II */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs()
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (11 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
---
unit/test-lib.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 171 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index 76e4363..a994dc2 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -227,6 +227,167 @@ static void test_sdp_get_lang_attr_invalid_dtd(void)
sdp_record_free(rec);
}
+static void test_sdp_get_profile_descs_valid(void)
+{
+ sdp_profile_desc_t profile;
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ sdp_uuid16_create(&profile.uuid, NAP_PROFILE_ID);
+ profile.version = 0x0100;
+ list = sdp_list_append(NULL, &profile);
+ err = sdp_set_profile_descs(rec, list);
+ sdp_list_free(list, NULL);
+ g_assert(err == 0);
+
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds[1], *values[1];
+ void *dtds2[] = { &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8()) */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UINT32)) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16)) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16, UINT32)) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_workaround(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT16, dtd3 = SDP_UINT32;
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ void *dtds[] = { &dtd1, &dtd2 };
+ void *values[] = { &u16, &u16 };
+ void *dtds2[] = { &dtd1, &dtd3 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT16) */
+ data = sdp_seq_alloc(dtds, values, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -247,5 +408,15 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_lang_attr/invalid_dtd",
test_sdp_get_lang_attr_invalid_dtd);
+ g_test_add_func("/lib/sdp_get_profile_descs/valid",
+ test_sdp_get_profile_descs_valid);
+ g_test_add_func("/lib/sdp_get_profile_descs/nodata",
+ test_sdp_get_profile_descs_nodata);
+ g_test_add_func("/lib/sdp_get_profile_descs/invalid_dtd",
+ test_sdp_get_profile_descs_invalid_dtd);
+ /* Test for workaround commented on sdp_get_profile_descs() */
+ g_test_add_func("/lib/sdp_get_profile_descs/workaround",
+ test_sdp_get_profile_descs_workaround);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver()
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (12 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
@ 2013-02-15 14:56 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
14 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 14:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
From: Anderson Lizardo <anderson.lizardo-430g2QfJUUCGglJvpFV4uA@public.gmane.org>
---
unit/test-lib.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index a994dc2..def133a 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -388,6 +388,53 @@ static void test_sdp_get_profile_descs_workaround(void)
sdp_record_free(rec);
}
+static void test_sdp_get_server_ver(void)
+{
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ void *dtds1[] = { &dtd1 };
+ void *values1[] = { &u16 };
+ void *dtds2[] = { &dtd2 };
+ void *values2[] = { &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ /* Valid DTD */
+ data = sdp_seq_alloc(dtds1, values1, 1);
+ sdp_attr_add(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == 0 && list != NULL);
+ sdp_list_free(list, NULL);
+
+ /* Invalid: UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8(UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -418,5 +465,7 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_profile_descs/workaround",
test_sdp_get_profile_descs_workaround);
+ g_test_add_func("/lib/sdp_get_server_ver", test_sdp_get_server_ver);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (13 preceding siblings ...)
2013-02-15 14:56 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
` (13 more replies)
14 siblings, 14 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Hi,
Change since v2:
* Fix commit author mangled during import from GMANE
Changes since v1:
* Fix license header to match BlueZ license (GPL v2 or later)
* Rename test source file and SDP tests to account for future addition of other
libbluetooth tests
This series adds various missing DTD validations, specially for SEQ* types. The
lack of these validations allows for a remote device to crash BlueZ due to
invalid memory access.
I also added unit tests for all affected functions. They are in a separate C
file (unit/test-lib.c), which will in future contain tests for other
libbluetooth API functions.
The only pending related fixes from my part are some missing NULL pointer
checks when accessing empty sequences. These will take some time to fix as they
affect profile code as well.
Best Regards,
Anderson Lizardo (13):
unit: Add initial SDP library unit tests
lib: Add SDP_IS_ALT() macro
lib: Reuse identical code in sdp_get_{add,}_access_protos()
lib: Cleanup coding style in sdp_get_proto_descs()
lib: Fix missing DTD validation while accessing SDP data elements
unit: Add tests for sdp_get_lang_attr()
lib: Add missing DTD validation in sdp_record_print()
lib: Validate DTDs when parsing LanguageBaseAttributeIDList
lib: Validate DTDs when parsing BluetoothProfileDescriptorList
lib: Add comment to BluetoothProfileDescriptorList parsing workaround
lib: Validate DTDs when parsing VersionNumberList
unit: Add tests for sdp_get_profile_descs()
unit: Add tests for sdp_get_server_ver()
.gitignore | 1 +
Makefile.am | 5 +
lib/sdp.c | 164 +++++++++++++------
lib/sdp.h | 1 +
unit/test-lib.c | 471 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 596 insertions(+), 46 deletions(-)
create mode 100644 unit/test-lib.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
` (12 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
These tests will cover all public SDP library API not covered yet by the
tests in test-sdp.c (which focus on the SDP server).
---
.gitignore | 1 +
Makefile.am | 5 ++
unit/test-lib.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 170 insertions(+)
create mode 100644 unit/test-lib.c
diff --git a/.gitignore b/.gitignore
index b818659..9483b92 100644
--- a/.gitignore
+++ b/.gitignore
@@ -65,6 +65,7 @@ unit/test-uuid
unit/test-textfile
unit/test-gdbus-client
unit/test-sdp
+unit/test-lib
unit/test-mgmt
tools/mgmt-tester
tools/gap-tester
diff --git a/Makefile.am b/Makefile.am
index 5ebf5e6..cbcde3e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -270,6 +270,11 @@ unit_test_gobex_apparam_SOURCES = $(gobex_sources) unit/util.c unit/util.h \
unit/test-gobex-apparam.c
unit_test_gobex_apparam_LDADD = @GLIB_LIBS@
+unit_tests += unit/test-lib
+
+unit_test_lib_SOURCES = unit/test-lib.c
+unit_test_lib_LDADD = lib/libbluetooth-private.la @GLIB_LIBS@
+
noinst_PROGRAMS += $(unit_tests)
TESTS = $(unit_tests)
diff --git a/unit/test-lib.c b/unit/test-lib.c
new file mode 100644
index 0000000..0be6d80
--- /dev/null
+++ b/unit/test-lib.c
@@ -0,0 +1,164 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
+ * Copyright (C) 2013 Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <glib.h>
+#include <stdlib.h>
+#include <errno.h>
+
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+
+static void test_sdp_get_access_protos_valid(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *aproto, *apseq, *proto[2];
+ const uint8_t u8 = 1;
+ uuid_t l2cap, rfcomm;
+ sdp_data_t *channel;
+ int err;
+
+ rec = sdp_record_alloc();
+ sdp_uuid16_create(&l2cap, L2CAP_UUID);
+ proto[0] = sdp_list_append(NULL, &l2cap);
+ apseq = sdp_list_append(NULL, proto[0]);
+
+ sdp_uuid16_create(&rfcomm, RFCOMM_UUID);
+ proto[1] = sdp_list_append(NULL, &rfcomm);
+ channel = sdp_data_alloc(SDP_UINT8, &u8);
+ proto[1] = sdp_list_append(proto[1], channel);
+ apseq = sdp_list_append(apseq, proto[1]);
+
+ aproto = sdp_list_append(NULL, apseq);
+ sdp_set_access_protos(rec, aproto);
+ sdp_set_add_access_protos(rec, aproto);
+ sdp_data_free(channel);
+ sdp_list_free(proto[0], NULL);
+ sdp_list_free(proto[1], NULL);
+ sdp_list_free(apseq, NULL);
+ sdp_list_free(aproto, NULL);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == 0);
+ sdp_list_foreach(aproto, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(aproto, NULL);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == 0);
+ sdp_list_foreach(aproto, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(aproto, NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_access_protos_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == ENODATA);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_access_protos_invalid_dtd1(void)
+{
+ const uint32_t u32 = 0xdeadbeeb;
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ sdp_data_t *data;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_PROTO_DESC_LIST, data);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_ADD_PROTO_DESC_LIST, data);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_access_protos_invalid_dtd2(void)
+{
+ uint8_t dtd = SDP_UINT8, u8 = 0xff;
+ void *dtds = &dtd, *values = &u8;
+ sdp_record_t *rec;
+ sdp_list_t *aproto;
+ sdp_data_t *data;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ data = sdp_seq_alloc(&dtds, &values, 1);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_PROTO_DESC_LIST, data);
+
+ err = sdp_get_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ data = sdp_seq_alloc(&dtds, &values, 1);
+ g_assert(data != NULL);
+ sdp_attr_replace(rec, SDP_ATTR_ADD_PROTO_DESC_LIST, data);
+
+ err = sdp_get_add_access_protos(rec, &aproto);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_func("/lib/sdp_get_access_protos/valid",
+ test_sdp_get_access_protos_valid);
+ g_test_add_func("/lib/sdp_get_access_protos/nodata",
+ test_sdp_get_access_protos_nodata);
+ g_test_add_func("/lib/sdp_get_access_protos/invalid_dtd1",
+ test_sdp_get_access_protos_invalid_dtd1);
+ g_test_add_func("/lib/sdp_get_access_protos/invalid_dtd2",
+ test_sdp_get_access_protos_invalid_dtd2);
+
+ return g_test_run();
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
` (11 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
This macro will simplify internal SDP library code, and is similar to
the existing SDP_IS_SEQ() macro.
---
lib/sdp.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/sdp.h b/lib/sdp.h
index a81e857..baae15f 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -440,6 +440,7 @@ typedef struct {
#define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || \
(x) == SDP_UUID128)
+#define SDP_IS_ALT(x) ((x) == SDP_ALT8 || (x) == SDP_ALT16 || (x) == SDP_ALT32)
#define SDP_IS_SEQ(x) ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
#define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
(x) == SDP_TEXT_STR32)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos()
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
` (10 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
sdp_get_access_protos() and sdp_get_add_access_protos() do almost
exactly the same thing, except for an additional statement for the
latter.
---
lib/sdp.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 8cfdde1..57abfd8 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1860,18 +1860,22 @@ sdp_data_t *sdp_get_proto_desc(sdp_list_t *list, int proto)
return NULL;
}
-int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
+ sdp_list_t **pap)
{
sdp_data_t *pdlist, *curr;
sdp_list_t *ap = 0;
- pdlist = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
+ pdlist = sdp_data_get(rec, attr_id);
if (pdlist == NULL) {
errno = ENODATA;
return -1;
}
SDPDBG("AP type : 0%x\n", pdlist->dtd);
+ if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
+ pdlist = pdlist->val.dataseq;
+
for (; pdlist; pdlist = pdlist->next) {
sdp_list_t *pds = 0;
for (curr = pdlist->val.dataseq; curr; curr = curr->next)
@@ -1882,28 +1886,14 @@ int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
return 0;
}
-int sdp_get_add_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
{
- sdp_data_t *pdlist, *curr;
- sdp_list_t *ap = 0;
-
- pdlist = sdp_data_get(rec, SDP_ATTR_ADD_PROTO_DESC_LIST);
- if (pdlist == NULL) {
- errno = ENODATA;
- return -1;
- }
- SDPDBG("AP type : 0%x\n", pdlist->dtd);
-
- pdlist = pdlist->val.dataseq;
+ return sdp_get_proto_descs(SDP_ATTR_PROTO_DESC_LIST, rec, pap);
+}
- for (; pdlist; pdlist = pdlist->next) {
- sdp_list_t *pds = 0;
- for (curr = pdlist->val.dataseq; curr; curr = curr->next)
- pds = sdp_list_append(pds, curr->val.dataseq);
- ap = sdp_list_append(ap, pds);
- }
- *pap = ap;
- return 0;
+int sdp_get_add_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
+{
+ return sdp_get_proto_descs(SDP_ATTR_ADD_PROTO_DESC_LIST, rec, pap);
}
int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs()
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (2 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
` (9 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
lib/sdp.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 57abfd8..3c45ac8 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1864,25 +1864,28 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
sdp_list_t **pap)
{
sdp_data_t *pdlist, *curr;
- sdp_list_t *ap = 0;
+ sdp_list_t *ap = NULL;
pdlist = sdp_data_get(rec, attr_id);
if (pdlist == NULL) {
errno = ENODATA;
return -1;
}
- SDPDBG("AP type : 0%x\n", pdlist->dtd);
+
+ SDPDBG("Attribute value type: 0x%02x\n", pdlist->dtd);
if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
pdlist = pdlist->val.dataseq;
for (; pdlist; pdlist = pdlist->next) {
- sdp_list_t *pds = 0;
+ sdp_list_t *pds = NULL;
for (curr = pdlist->val.dataseq; curr; curr = curr->next)
pds = sdp_list_append(pds, curr->val.dataseq);
ap = sdp_list_append(ap, pds);
}
+
*pap = ap;
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (3 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
` (8 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
It is necessary to validate the sdp_data_t "dtd" field before accessing
the "val" union members, specially when handling SDP_SEQ*, SDP_ALT* and
SDP_STR* elements, otherwise remote devices can trigger memory
corruption by passing invalid data elements where others are expected.
---
lib/sdp.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 3c45ac8..9c1eeb5 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1874,19 +1874,41 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
SDPDBG("Attribute value type: 0x%02x\n", pdlist->dtd);
- if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST)
+ if (attr_id == SDP_ATTR_ADD_PROTO_DESC_LIST) {
+ if (!SDP_IS_SEQ(pdlist->dtd)) {
+ errno = EINVAL;
+ return -1;
+ }
pdlist = pdlist->val.dataseq;
+ }
for (; pdlist; pdlist = pdlist->next) {
sdp_list_t *pds = NULL;
- for (curr = pdlist->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(pdlist->dtd) && !SDP_IS_ALT(pdlist->dtd))
+ goto failed;
+
+ for (curr = pdlist->val.dataseq; curr; curr = curr->next) {
+ if (!SDP_IS_SEQ(curr->dtd)) {
+ sdp_list_free(pds, NULL);
+ goto failed;
+ }
pds = sdp_list_append(pds, curr->val.dataseq);
+ }
+
ap = sdp_list_append(ap, pds);
}
*pap = ap;
return 0;
+
+failed:
+ sdp_list_foreach(ap, (sdp_list_func_t) sdp_list_free, NULL);
+ sdp_list_free(ap, NULL);
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_access_protos(const sdp_record_t *rec, sdp_list_t **pap)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr()
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (4 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
` (7 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
unit/test-lib.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index 0be6d80..76e4363 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -147,6 +147,86 @@ static void test_sdp_get_access_protos_invalid_dtd2(void)
sdp_record_free(rec);
}
+static void test_sdp_get_lang_attr_valid(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+ sdp_add_lang_attr(rec);
+
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == 0);
+
+ sdp_list_free(list, free);
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_lang_attr_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_lang_attr_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds1[] = { &dtd1, &dtd2, &dtd2 };
+ void *values1[] = { &u16, &u32, &u32 };
+ void *dtds2[] = { &dtd1, &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds1[1], &values1[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT16) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT32, UINT32) */
+ data = sdp_seq_alloc(dtds1, values1, 3);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT16, UINT16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 3);
+ sdp_attr_replace(rec, SDP_ATTR_LANG_BASE_ATTR_ID_LIST, data);
+ err = sdp_get_lang_attr(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -160,5 +240,12 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_access_protos/invalid_dtd2",
test_sdp_get_access_protos_invalid_dtd2);
+ g_test_add_func("/lib/sdp_get_lang_attr/valid",
+ test_sdp_get_lang_attr_valid);
+ g_test_add_func("/lib/sdp_get_lang_attr/nodata",
+ test_sdp_get_lang_attr_nodata);
+ g_test_add_func("/lib/sdp_get_lang_attr/invalid_dtd",
+ test_sdp_get_lang_attr_invalid_dtd);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print()
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (5 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
` (6 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
lib/sdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 9c1eeb5..782d9bd 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1603,13 +1603,13 @@ static void print_dataseq(sdp_data_t *p)
void sdp_record_print(const sdp_record_t *rec)
{
sdp_data_t *d = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Name: %.*s\n", d->unitSize, d->val.str);
d = sdp_data_get(rec, SDP_ATTR_SVCDESC_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Description: %.*s\n", d->unitSize, d->val.str);
d = sdp_data_get(rec, SDP_ATTR_PROVNAME_PRIMARY);
- if (d)
+ if (d && SDP_IS_TEXT_STR(d->dtd))
printf("Service Provider: %.*s\n", d->unitSize, d->val.str);
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (6 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
` (5 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Also check if the required number of entries is present.
---
lib/sdp.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 782d9bd..77dafaa 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2019,19 +2019,30 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
errno = ENODATA;
return -1;
}
+
+ if (!SDP_IS_SEQ(sdpdata->dtd))
+ goto invalid;
curr_data = sdpdata->val.dataseq;
+
while (curr_data) {
- sdp_data_t *pCode = curr_data;
- sdp_data_t *pEncoding;
- sdp_data_t *pOffset;
+ sdp_data_t *pCode, *pEncoding, *pOffset;
+
+ pCode = curr_data;
+ if (pCode->dtd != SDP_UINT16)
+ goto invalid;
+
+ /* LanguageBaseAttributeIDList entries are always grouped as
+ * triplets */
+ if (!pCode->next || !pCode->next->next)
+ goto invalid;
pEncoding = pCode->next;
- if (!pEncoding)
- break;
+ if (pEncoding->dtd != SDP_UINT16)
+ goto invalid;
pOffset = pEncoding->next;
- if (!pOffset)
- break;
+ if (pOffset->dtd != SDP_UINT16)
+ goto invalid;
lang = malloc(sizeof(sdp_lang_attr_t));
if (!lang) {
@@ -2051,6 +2062,13 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
}
return 0;
+
+invalid:
+ sdp_list_free(*langSeq, free);
+ *langSeq = NULL;
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (7 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
` (4 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
The "seq->val.dataseq != NULL" check is also removed from the for()
statement because it should be done after verifying that the data
element is a sequence (inside the "if (SDP_IS_SEQ(...))" block.)
---
lib/sdp.c | 37 ++++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 77dafaa..396567f 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2078,11 +2078,15 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
*profDescSeq = NULL;
sdpdata = sdp_data_get(rec, SDP_ATTR_PFILE_DESC_LIST);
- if (!sdpdata || !sdpdata->val.dataseq) {
+ if (sdpdata == NULL) {
errno = ENODATA;
return -1;
}
- for (seq = sdpdata->val.dataseq; seq && seq->val.dataseq; seq = seq->next) {
+
+ if (!SDP_IS_SEQ(sdpdata->dtd) || sdpdata->val.dataseq == NULL)
+ goto invalid;
+
+ for (seq = sdpdata->val.dataseq; seq; seq = seq->next) {
uuid_t *uuid = NULL;
uint16_t version = 0x100;
@@ -2094,13 +2098,21 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
seq = next;
}
} else if (SDP_IS_SEQ(seq->dtd)) {
- sdp_data_t *puuid = seq->val.dataseq;
- sdp_data_t *pVnum = seq->val.dataseq->next;
- if (puuid && pVnum) {
- uuid = &puuid->val.uuid;
- version = pVnum->val.uint16;
- }
- }
+ sdp_data_t *puuid, *pVnum;
+
+ puuid = seq->val.dataseq;
+ if (puuid == NULL || !SDP_IS_UUID(puuid->dtd))
+ goto invalid;
+
+ uuid = &puuid->val.uuid;
+
+ pVnum = puuid->next;
+ if (pVnum == NULL || pVnum->dtd != SDP_UINT16)
+ goto invalid;
+
+ version = pVnum->val.uint16;
+ } else
+ goto invalid;
if (uuid != NULL) {
profDesc = malloc(sizeof(sdp_profile_desc_t));
@@ -2119,6 +2131,13 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
}
}
return 0;
+
+invalid:
+ sdp_list_free(*profDescSeq, free);
+ *profDescSeq = NULL;
+ errno = EINVAL;
+
+ return -1;
}
int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (8 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
` (3 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
Commits 0f5a5a9580084a3c4e0644ef5cd75689aeb5ff40 and
46b3a3d2d00bf70bc57ef0c9ad5542a2271e3350 introduced this workaround.
---
lib/sdp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index 396567f..61598bb 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2091,6 +2091,11 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
uint16_t version = 0x100;
if (SDP_IS_UUID(seq->dtd)) {
+ /* Mac OS X 10.7.3 and old Samsung phones do not comply
+ * to the SDP specification for
+ * BluetoothProfileDescriptorList. This workaround
+ * allows to properly parse UUID/version from SDP
+ * record published by these systems. */
sdp_data_t *next = seq->next;
uuid = &seq->val.uuid;
if (next && next->dtd == SDP_UINT16) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (9 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
` (2 subsequent siblings)
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
lib/sdp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 61598bb..6c73818 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2155,9 +2155,24 @@ int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
errno = ENODATA;
return -1;
}
- for (curr = d->val.dataseq; curr; curr = curr->next)
+
+ if (!SDP_IS_SEQ(d->dtd) || d->val.dataseq == NULL)
+ goto invalid;
+
+ for (curr = d->val.dataseq; curr; curr = curr->next) {
+ if (curr->dtd != SDP_UINT16)
+ goto invalid;
*u16 = sdp_list_append(*u16, &curr->val.uint16);
+ }
+
return 0;
+
+invalid:
+ sdp_list_free(*u16, NULL);
+ *u16 = NULL;
+ errno = EINVAL;
+
+ return -1;
}
/* flexible extraction of basic attributes - Jean II */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs()
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (10 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
2013-02-18 8:05 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Johan Hedberg
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
unit/test-lib.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 171 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index 76e4363..a994dc2 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -227,6 +227,167 @@ static void test_sdp_get_lang_attr_invalid_dtd(void)
sdp_record_free(rec);
}
+static void test_sdp_get_profile_descs_valid(void)
+{
+ sdp_profile_desc_t profile;
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ sdp_uuid16_create(&profile.uuid, NAP_PROFILE_ID);
+ profile.version = 0x0100;
+ list = sdp_list_append(NULL, &profile);
+ err = sdp_set_profile_descs(rec, list);
+ sdp_list_free(list, NULL);
+ g_assert(err == 0);
+
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_nodata(void)
+{
+ sdp_record_t *rec;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_invalid_dtd(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT32;
+ uint32_t u32 = 0xdeadbeeb;
+ uint16_t u16 = 0x1234;
+ void *dtds[1], *values[1];
+ void *dtds2[] = { &dtd1, &dtd2 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ g_assert(data != NULL);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(UINT32) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8()) */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UINT32)) */
+ data = sdp_seq_alloc(&dtds2[1], &values2[1], 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16)) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* SEQ8(SEQ8(UUID16, UINT32)) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ dtds[0] = &data->dtd;
+ values[0] = data;
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
+static void test_sdp_get_profile_descs_workaround(void)
+{
+ uint8_t dtd1 = SDP_UUID16, dtd2 = SDP_UINT16, dtd3 = SDP_UINT32;
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ void *dtds[] = { &dtd1, &dtd2 };
+ void *values[] = { &u16, &u16 };
+ void *dtds2[] = { &dtd1, &dtd3 };
+ void *values2[] = { &u16, &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_add(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT16) */
+ data = sdp_seq_alloc(dtds, values, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16) */
+ data = sdp_seq_alloc(dtds, values, 1);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ sdp_list_free(list, free);
+ g_assert(err == 0 && list != NULL);
+
+ /* SEQ8(UUID16, UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 2);
+ sdp_attr_replace(rec, SDP_ATTR_PFILE_DESC_LIST, data);
+ list = NULL;
+ err = sdp_get_profile_descs(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -247,5 +408,15 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_lang_attr/invalid_dtd",
test_sdp_get_lang_attr_invalid_dtd);
+ g_test_add_func("/lib/sdp_get_profile_descs/valid",
+ test_sdp_get_profile_descs_valid);
+ g_test_add_func("/lib/sdp_get_profile_descs/nodata",
+ test_sdp_get_profile_descs_nodata);
+ g_test_add_func("/lib/sdp_get_profile_descs/invalid_dtd",
+ test_sdp_get_profile_descs_invalid_dtd);
+ /* Test for workaround commented on sdp_get_profile_descs() */
+ g_test_add_func("/lib/sdp_get_profile_descs/workaround",
+ test_sdp_get_profile_descs_workaround);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver()
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (11 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
@ 2013-02-15 15:07 ` Anderson Lizardo
2013-02-18 8:05 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Johan Hedberg
13 siblings, 0 replies; 44+ messages in thread
From: Anderson Lizardo @ 2013-02-15 15:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
---
unit/test-lib.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/unit/test-lib.c b/unit/test-lib.c
index a994dc2..def133a 100644
--- a/unit/test-lib.c
+++ b/unit/test-lib.c
@@ -388,6 +388,53 @@ static void test_sdp_get_profile_descs_workaround(void)
sdp_record_free(rec);
}
+static void test_sdp_get_server_ver(void)
+{
+ uint16_t u16 = 0x1234;
+ uint32_t u32 = 0xdeadbeeb;
+ uint8_t dtd1 = SDP_UINT16, dtd2 = SDP_UINT32;
+ void *dtds1[] = { &dtd1 };
+ void *values1[] = { &u16 };
+ void *dtds2[] = { &dtd2 };
+ void *values2[] = { &u32 };
+ sdp_record_t *rec;
+ sdp_data_t *data;
+ sdp_list_t *list;
+ int err;
+
+ rec = sdp_record_alloc();
+
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == ENODATA);
+
+ /* Valid DTD */
+ data = sdp_seq_alloc(dtds1, values1, 1);
+ sdp_attr_add(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == 0 && list != NULL);
+ sdp_list_free(list, NULL);
+
+ /* Invalid: UINT32 */
+ data = sdp_data_alloc(SDP_UINT32, &u32);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8() */
+ data = sdp_seq_alloc(NULL, NULL, 0);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ /* Invalid: SEQ8(UINT32) */
+ data = sdp_seq_alloc(dtds2, values2, 1);
+ sdp_attr_replace(rec, SDP_ATTR_VERSION_NUM_LIST, data);
+ err = sdp_get_server_ver(rec, &list);
+ g_assert(err == -1 && errno == EINVAL);
+
+ sdp_record_free(rec);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -418,5 +465,7 @@ int main(int argc, char *argv[])
g_test_add_func("/lib/sdp_get_profile_descs/workaround",
test_sdp_get_profile_descs_workaround);
+ g_test_add_func("/lib/sdp_get_server_ver", test_sdp_get_server_ver);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
` (12 preceding siblings ...)
2013-02-15 15:07 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
@ 2013-02-18 8:05 ` Johan Hedberg
13 siblings, 0 replies; 44+ messages in thread
From: Johan Hedberg @ 2013-02-18 8:05 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
Hi Lizardo,
On Fri, Feb 15, 2013, Anderson Lizardo wrote:
> Change since v2:
> * Fix commit author mangled during import from GMANE
>
> Changes since v1:
> * Fix license header to match BlueZ license (GPL v2 or later)
> * Rename test source file and SDP tests to account for future addition of other
> libbluetooth tests
>
> This series adds various missing DTD validations, specially for SEQ* types. The
> lack of these validations allows for a remote device to crash BlueZ due to
> invalid memory access.
>
> I also added unit tests for all affected functions. They are in a separate C
> file (unit/test-lib.c), which will in future contain tests for other
> libbluetooth API functions.
>
> The only pending related fixes from my part are some missing NULL pointer
> checks when accessing empty sequences. These will take some time to fix as they
> affect profile code as well.
>
> Best Regards,
>
> Anderson Lizardo (13):
> unit: Add initial SDP library unit tests
> lib: Add SDP_IS_ALT() macro
> lib: Reuse identical code in sdp_get_{add,}_access_protos()
> lib: Cleanup coding style in sdp_get_proto_descs()
> lib: Fix missing DTD validation while accessing SDP data elements
> unit: Add tests for sdp_get_lang_attr()
> lib: Add missing DTD validation in sdp_record_print()
> lib: Validate DTDs when parsing LanguageBaseAttributeIDList
> lib: Validate DTDs when parsing BluetoothProfileDescriptorList
> lib: Add comment to BluetoothProfileDescriptorList parsing workaround
> lib: Validate DTDs when parsing VersionNumberList
> unit: Add tests for sdp_get_profile_descs()
> unit: Add tests for sdp_get_server_ver()
>
> .gitignore | 1 +
> Makefile.am | 5 +
> lib/sdp.c | 164 +++++++++++++------
> lib/sdp.h | 1 +
> unit/test-lib.c | 471 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 596 insertions(+), 46 deletions(-)
> create mode 100644 unit/test-lib.c
All patches in this set have been applied. Thanks.
Johan
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2013-02-18 8:05 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-10 21:26 [PATCH BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
2013-02-10 21:26 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
2013-02-15 14:56 ` [PATCH v2 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-15 14:51 ` Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
2013-02-15 14:56 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
2013-02-15 15:07 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 01/13] unit: Add initial SDP library unit tests Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 02/13] lib: Add SDP_IS_ALT() macro Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 03/13] lib: Reuse identical code in sdp_get_{add,}_access_protos() Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 04/13] lib: Cleanup coding style in sdp_get_proto_descs() Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 05/13] lib: Fix missing DTD validation while accessing SDP data elements Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 06/13] unit: Add tests for sdp_get_lang_attr() Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 07/13] lib: Add missing DTD validation in sdp_record_print() Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 08/13] lib: Validate DTDs when parsing LanguageBaseAttributeIDList Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 09/13] lib: Validate DTDs when parsing BluetoothProfileDescriptorList Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 10/13] lib: Add comment to BluetoothProfileDescriptorList parsing workaround Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 11/13] lib: Validate DTDs when parsing VersionNumberList Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 12/13] unit: Add tests for sdp_get_profile_descs() Anderson Lizardo
2013-02-15 15:07 ` [PATCH BlueZ 13/13] unit: Add tests for sdp_get_server_ver() Anderson Lizardo
2013-02-18 8:05 ` [PATCH v3 BlueZ 00/13] Fix SDP DE Type Descriptor validation issues Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox