* [PATCH 0/3 BlueZ] hcidump: fix buffer overflow
@ 2018-10-31 8:15 Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow Cho, Yu-Chen
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Cho, Yu-Chen @ 2018-10-31 8:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: acho, jlee
Bugs were simple buffer overflow that was actually discovered already couple years ago
by op7ic:https://www.spinics.net/lists/linux-bluetooth/msg68892.html
Caused by missing boundary checks before accessing.
Cho, Yu-Chen (3):
hcidump: fixed AMP Assoc dump heap-over-flow
hcidump:fixed hci frame dump stack-buffer-overflow
hcidump: Fix set_ext_ctrl() global buffer overflow
tools/parser/amp.c | 65 +++++++++++++++++++++++---------------------
tools/parser/hci.c | 3 ++
tools/parser/l2cap.c | 2 +-
3 files changed, 38 insertions(+), 32 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow
2018-10-31 8:15 [PATCH 0/3 BlueZ] hcidump: fix buffer overflow Cho, Yu-Chen
@ 2018-10-31 8:15 ` Cho, Yu-Chen
2018-11-02 10:54 ` Luiz Augusto von Dentz
2018-10-31 8:15 ` [PATCH 2/3 BlueZ] hcidump:fixed hci frame dump stack-buffer-overflow Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 3/3 BlueZ] hcidump: Fix set_ext_ctrl() global buffer overflow Cho, Yu-Chen
2 siblings, 1 reply; 8+ messages in thread
From: Cho, Yu-Chen @ 2018-10-31 8:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: acho, jlee
amp_assoc_dump() didn't check the length of amp assoc struct of
Type-Length-Value (TLV) triplets, and the Connected Chan List
(number of triplets) is also need to check, or there are wrong
length for the number of triplets.
Signed-off-by: Cho, Yu-Chen <acho@suse.com>
---
tools/parser/amp.c | 65 ++++++++++++++++++++++++----------------------
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/tools/parser/amp.c b/tools/parser/amp.c
index 158ca4a75..bd7f91000 100644
--- a/tools/parser/amp.c
+++ b/tools/parser/amp.c
@@ -33,7 +33,7 @@ static void amp_dump_chanlist(int level, struct amp_tlv *tlv, char *prefix)
struct amp_country_triplet *triplet;
int i, num;
- num = (tlv->len - sizeof(*chan_list)) / sizeof(*triplet);
+ num = sizeof(*chan_list->triplets) / sizeof(*chan_list->triplets[0]);
printf("%s (number of triplets %d)\n", prefix, num);
@@ -80,47 +80,50 @@ void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len)
p_indent(level+1, 0);
- switch (tlv->type) {
- case A2MP_MAC_ADDR_TYPE:
- if (tlvlen != 6)
- break;
- printf("MAC: %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
+ if (tlvlen !=0) {
+ switch (tlv->type) {
+ case A2MP_MAC_ADDR_TYPE:
+ if (tlvlen != 6)
+ break;
+ printf("MAC: %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
tlv->val[0], tlv->val[1], tlv->val[2],
tlv->val[3], tlv->val[4], tlv->val[5]);
- break;
-
- case A2MP_PREF_CHANLIST_TYPE:
- amp_dump_chanlist(level, tlv, "Preferred Chan List");
- break;
+ break;
- case A2MP_CONNECTED_CHAN:
- amp_dump_chanlist(level, tlv, "Connected Chan List");
- break;
+ case A2MP_PREF_CHANLIST_TYPE:
+ amp_dump_chanlist(level, tlv, "Preferred Chan List");
+ break;
- case A2MP_PAL_CAP_TYPE:
- if (tlvlen != 4)
+ case A2MP_CONNECTED_CHAN:
+ amp_dump_chanlist(level, tlv, "Connected Chan List");
break;
- printf("PAL CAP: %2.2x %2.2x %2.2x %2.2x\n",
+
+ case A2MP_PAL_CAP_TYPE:
+ if (tlvlen != 4)
+ break;
+ printf("PAL CAP: %2.2x %2.2x %2.2x %2.2x\n",
tlv->val[0], tlv->val[1], tlv->val[2],
tlv->val[3]);
- break;
-
- case A2MP_PAL_VER_INFO:
- if (tlvlen != 5)
break;
- ver = (struct amp_pal_ver *) tlv->val;
- printf("PAL VER: %2.2x Comp ID: %4.4x SubVer: %4.4x\n",
+
+ case A2MP_PAL_VER_INFO:
+ if (tlvlen != 5)
+ break;
+ ver = (struct amp_pal_ver *) tlv->val;
+ printf("PAL VER: %2.2x Comp ID: %4.4x SubVer: %4.4x\n",
ver->ver, btohs(ver->company_id),
btohs(ver->sub_ver));
- break;
+ break;
- default:
- printf("Unrecognized type %d\n", tlv->type);
- break;
- }
+ default:
+ printf("Unrecognized type %d\n", tlv->type);
+ break;
+ }
- len -= tlvlen + sizeof(*tlv);
- assoc += tlvlen + sizeof(*tlv);
- tlv = (struct amp_tlv *) assoc;
+ len -= tlvlen + sizeof(*tlv);
+ assoc += tlvlen + sizeof(*tlv);
+ tlv = (struct amp_tlv *) assoc;
+ } else
+ break;
}
}
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3 BlueZ] hcidump:fixed hci frame dump stack-buffer-overflow
2018-10-31 8:15 [PATCH 0/3 BlueZ] hcidump: fix buffer overflow Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow Cho, Yu-Chen
@ 2018-10-31 8:15 ` Cho, Yu-Chen
2018-11-02 11:11 ` Luiz Augusto von Dentz
2018-10-31 8:15 ` [PATCH 3/3 BlueZ] hcidump: Fix set_ext_ctrl() global buffer overflow Cho, Yu-Chen
2 siblings, 1 reply; 8+ messages in thread
From: Cho, Yu-Chen @ 2018-10-31 8:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: acho, jlee
hci_dump() didn't check the length of frame, and it would be
a stack-buffer-overflow error.
Signed-off-by: Cho, Yu-Chen <acho@suse.com>
---
tools/parser/hci.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/parser/hci.c b/tools/parser/hci.c
index 8c7bd2581..4e6c36040 100644
--- a/tools/parser/hci.c
+++ b/tools/parser/hci.c
@@ -4107,6 +4107,9 @@ void hci_dump(int level, struct frame *frm)
frm->ptr++; frm->len--;
+ if (frm->len == 0)
+ return;
+
switch (type) {
case HCI_COMMAND_PKT:
command_dump(level, frm);
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3 BlueZ] hcidump: Fix set_ext_ctrl() global buffer overflow
2018-10-31 8:15 [PATCH 0/3 BlueZ] hcidump: fix buffer overflow Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 2/3 BlueZ] hcidump:fixed hci frame dump stack-buffer-overflow Cho, Yu-Chen
@ 2018-10-31 8:15 ` Cho, Yu-Chen
2018-11-02 11:12 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 8+ messages in thread
From: Cho, Yu-Chen @ 2018-10-31 8:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: acho, jlee
Fix set_ext_ctrl() global buffer overflow.
Signed-off-by: Cho, Yu-Chen <acho@suse.com>
---
tools/parser/l2cap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/parser/l2cap.c b/tools/parser/l2cap.c
index a05796482..5daefcbaa 100644
--- a/tools/parser/l2cap.c
+++ b/tools/parser/l2cap.c
@@ -56,7 +56,7 @@ typedef struct {
uint8_t mode;
uint8_t ext_ctrl;
} cid_info;
-#define CID_TABLE_SIZE 20
+#define CID_TABLE_SIZE 32
static cid_info cid_table[2][CID_TABLE_SIZE];
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow
2018-10-31 8:15 ` [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow Cho, Yu-Chen
@ 2018-11-02 10:54 ` Luiz Augusto von Dentz
2018-11-06 6:57 ` Cho, Yu-Chen
0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-02 10:54 UTC (permalink / raw)
To: Cho, Yu-Chen; +Cc: linux-bluetooth, jlee
Hi,
On Wed, Oct 31, 2018 at 11:08 AM Cho, Yu-Chen <acho@suse.com> wrote:
>
> amp_assoc_dump() didn't check the length of amp assoc struct of
> Type-Length-Value (TLV) triplets, and the Connected Chan List
> (number of triplets) is also need to check, or there are wrong
> length for the number of triplets.
>
> Signed-off-by: Cho, Yu-Chen <acho@suse.com>
Please remove the Signed-off-by, we don't use that in userspace.
> ---
> tools/parser/amp.c | 65 ++++++++++++++++++++++++----------------------
> 1 file changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/tools/parser/amp.c b/tools/parser/amp.c
> index 158ca4a75..bd7f91000 100644
> --- a/tools/parser/amp.c
> +++ b/tools/parser/amp.c
> @@ -33,7 +33,7 @@ static void amp_dump_chanlist(int level, struct amp_tlv *tlv, char *prefix)
> struct amp_country_triplet *triplet;
> int i, num;
>
> - num = (tlv->len - sizeof(*chan_list)) / sizeof(*triplet);
> + num = sizeof(*chan_list->triplets) / sizeof(*chan_list->triplets[0]);
>
> printf("%s (number of triplets %d)\n", prefix, num);
>
> @@ -80,47 +80,50 @@ void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len)
>
> p_indent(level+1, 0);
>
> - switch (tlv->type) {
> - case A2MP_MAC_ADDR_TYPE:
> - if (tlvlen != 6)
> - break;
> - printf("MAC: %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
> + if (tlvlen !=0) {
> + switch (tlv->type) {
> + case A2MP_MAC_ADDR_TYPE:
> + if (tlvlen != 6)
> + break;
> + printf("MAC: %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
> tlv->val[0], tlv->val[1], tlv->val[2],
> tlv->val[3], tlv->val[4], tlv->val[5]);
> - break;
> -
> - case A2MP_PREF_CHANLIST_TYPE:
> - amp_dump_chanlist(level, tlv, "Preferred Chan List");
> - break;
> + break;
>
> - case A2MP_CONNECTED_CHAN:
> - amp_dump_chanlist(level, tlv, "Connected Chan List");
> - break;
> + case A2MP_PREF_CHANLIST_TYPE:
> + amp_dump_chanlist(level, tlv, "Preferred Chan List");
> + break;
>
> - case A2MP_PAL_CAP_TYPE:
> - if (tlvlen != 4)
> + case A2MP_CONNECTED_CHAN:
> + amp_dump_chanlist(level, tlv, "Connected Chan List");
> break;
> - printf("PAL CAP: %2.2x %2.2x %2.2x %2.2x\n",
> +
> + case A2MP_PAL_CAP_TYPE:
> + if (tlvlen != 4)
> + break;
> + printf("PAL CAP: %2.2x %2.2x %2.2x %2.2x\n",
> tlv->val[0], tlv->val[1], tlv->val[2],
> tlv->val[3]);
> - break;
> -
> - case A2MP_PAL_VER_INFO:
> - if (tlvlen != 5)
> break;
> - ver = (struct amp_pal_ver *) tlv->val;
> - printf("PAL VER: %2.2x Comp ID: %4.4x SubVer: %4.4x\n",
> +
> + case A2MP_PAL_VER_INFO:
> + if (tlvlen != 5)
> + break;
> + ver = (struct amp_pal_ver *) tlv->val;
> + printf("PAL VER: %2.2x Comp ID: %4.4x SubVer: %4.4x\n",
> ver->ver, btohs(ver->company_id),
> btohs(ver->sub_ver));
> - break;
> + break;
>
> - default:
> - printf("Unrecognized type %d\n", tlv->type);
> - break;
> - }
> + default:
> + printf("Unrecognized type %d\n", tlv->type);
> + break;
> + }
>
> - len -= tlvlen + sizeof(*tlv);
> - assoc += tlvlen + sizeof(*tlv);
> - tlv = (struct amp_tlv *) assoc;
> + len -= tlvlen + sizeof(*tlv);
> + assoc += tlvlen + sizeof(*tlv);
> + tlv = (struct amp_tlv *) assoc;
> + } else
> + break;
> }
> }
> --
> 2.19.1
Please fix these:
Applying: hcidump: fixed AMP Assoc dump heap-over-flow
WARNING:LONG_LINE_STRING: line over 80 characters
#28: FILE: tools/parser/amp.c:88:
+ printf("MAC: %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
WARNING:LONG_LINE_STRING: line over 80 characters
#42: FILE: tools/parser/amp.c:94:
+ amp_dump_chanlist(level, tlv, "Preferred Chan List");
WARNING:LONG_LINE_STRING: line over 80 characters
#48: FILE: tools/parser/amp.c:98:
+ amp_dump_chanlist(level, tlv, "Connected Chan List");
WARNING:LONG_LINE_STRING: line over 80 characters
#70: FILE: tools/parser/amp.c:113:
+ printf("PAL VER: %2.2x Comp ID: %4.4x SubVer: %4.4x\n",
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3 BlueZ] hcidump:fixed hci frame dump stack-buffer-overflow
2018-10-31 8:15 ` [PATCH 2/3 BlueZ] hcidump:fixed hci frame dump stack-buffer-overflow Cho, Yu-Chen
@ 2018-11-02 11:11 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-02 11:11 UTC (permalink / raw)
To: Cho, Yu-Chen; +Cc: linux-bluetooth, jlee
Hi,
On Wed, Oct 31, 2018 at 10:39 AM Cho, Yu-Chen <acho@suse.com> wrote:
>
> hci_dump() didn't check the length of frame, and it would be
> a stack-buffer-overflow error.
>
> Signed-off-by: Cho, Yu-Chen <acho@suse.com>
> ---
> tools/parser/hci.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/parser/hci.c b/tools/parser/hci.c
> index 8c7bd2581..4e6c36040 100644
> --- a/tools/parser/hci.c
> +++ b/tools/parser/hci.c
> @@ -4107,6 +4107,9 @@ void hci_dump(int level, struct frame *frm)
>
> frm->ptr++; frm->len--;
>
> + if (frm->len == 0)
> + return;
> +
> switch (type) {
> case HCI_COMMAND_PKT:
> command_dump(level, frm);
> --
> 2.19.1
Applied.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3 BlueZ] hcidump: Fix set_ext_ctrl() global buffer overflow
2018-10-31 8:15 ` [PATCH 3/3 BlueZ] hcidump: Fix set_ext_ctrl() global buffer overflow Cho, Yu-Chen
@ 2018-11-02 11:12 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2018-11-02 11:12 UTC (permalink / raw)
To: Cho, Yu-Chen; +Cc: linux-bluetooth, jlee
Hi,
On Wed, Oct 31, 2018 at 10:38 AM Cho, Yu-Chen <acho@suse.com> wrote:
>
> Fix set_ext_ctrl() global buffer overflow.
>
> Signed-off-by: Cho, Yu-Chen <acho@suse.com>
> ---
> tools/parser/l2cap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/parser/l2cap.c b/tools/parser/l2cap.c
> index a05796482..5daefcbaa 100644
> --- a/tools/parser/l2cap.c
> +++ b/tools/parser/l2cap.c
> @@ -56,7 +56,7 @@ typedef struct {
> uint8_t mode;
> uint8_t ext_ctrl;
> } cid_info;
> -#define CID_TABLE_SIZE 20
> +#define CID_TABLE_SIZE 32
>
> static cid_info cid_table[2][CID_TABLE_SIZE];
>
> --
> 2.19.1
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow
2018-11-02 10:54 ` Luiz Augusto von Dentz
@ 2018-11-06 6:57 ` Cho, Yu-Chen
0 siblings, 0 replies; 8+ messages in thread
From: Cho, Yu-Chen @ 2018-11-06 6:57 UTC (permalink / raw)
To: luiz.dentz; +Cc: linux-bluetooth, acho, jlee
amp_assoc_dump() didn't check the length of amp assoc struct of
Type-Length-Value (TLV) triplets, and the Connected Chan List
(number of triplets) is also need to check, or there are wrong
length for the number of triplets.
Signed-off-by: Cho, Yu-Chen <acho@suse.com>
---
tools/parser/amp.c | 69 +++++++++++++++++++++++++---------------------
1 file changed, 38 insertions(+), 31 deletions(-)
diff --git a/tools/parser/amp.c b/tools/parser/amp.c
index 158ca4a75..420099c90 100644
--- a/tools/parser/amp.c
+++ b/tools/parser/amp.c
@@ -33,7 +33,7 @@ static void amp_dump_chanlist(int level, struct amp_tlv *tlv, char *prefix)
struct amp_country_triplet *triplet;
int i, num;
- num = (tlv->len - sizeof(*chan_list)) / sizeof(*triplet);
+ num = sizeof(*chan_list->triplets) / sizeof(*chan_list->triplets[0]);
printf("%s (number of triplets %d)\n", prefix, num);
@@ -80,47 +80,54 @@ void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len)
p_indent(level+1, 0);
- switch (tlv->type) {
- case A2MP_MAC_ADDR_TYPE:
- if (tlvlen != 6)
- break;
- printf("MAC: %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
+ if (tlvlen != 0) {
+ switch (tlv->type) {
+ case A2MP_MAC_ADDR_TYPE:
+ if (tlvlen != 6)
+ break;
+ printf("MAC: ");
+ printf("%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
tlv->val[0], tlv->val[1], tlv->val[2],
tlv->val[3], tlv->val[4], tlv->val[5]);
- break;
-
- case A2MP_PREF_CHANLIST_TYPE:
- amp_dump_chanlist(level, tlv, "Preferred Chan List");
- break;
+ break;
- case A2MP_CONNECTED_CHAN:
- amp_dump_chanlist(level, tlv, "Connected Chan List");
- break;
+ case A2MP_PREF_CHANLIST_TYPE:
+ amp_dump_chanlist(level, tlv,
+ "Preferred Chan List");
+ break;
- case A2MP_PAL_CAP_TYPE:
- if (tlvlen != 4)
+ case A2MP_CONNECTED_CHAN:
+ amp_dump_chanlist(level, tlv,
+ "Connected Chan List");
break;
- printf("PAL CAP: %2.2x %2.2x %2.2x %2.2x\n",
+
+ case A2MP_PAL_CAP_TYPE:
+ if (tlvlen != 4)
+ break;
+ printf("PAL CAP: %2.2x %2.2x %2.2x %2.2x\n",
tlv->val[0], tlv->val[1], tlv->val[2],
tlv->val[3]);
- break;
-
- case A2MP_PAL_VER_INFO:
- if (tlvlen != 5)
break;
- ver = (struct amp_pal_ver *) tlv->val;
- printf("PAL VER: %2.2x Comp ID: %4.4x SubVer: %4.4x\n",
+
+ case A2MP_PAL_VER_INFO:
+ if (tlvlen != 5)
+ break;
+ ver = (struct amp_pal_ver *) tlv->val;
+ printf("PAL VER: ");
+ printf("%2.2x Comp ID: %4.4x SubVer: %4.4x\n",
ver->ver, btohs(ver->company_id),
btohs(ver->sub_ver));
- break;
+ break;
- default:
- printf("Unrecognized type %d\n", tlv->type);
- break;
- }
+ default:
+ printf("Unrecognized type %d\n", tlv->type);
+ break;
+ }
- len -= tlvlen + sizeof(*tlv);
- assoc += tlvlen + sizeof(*tlv);
- tlv = (struct amp_tlv *) assoc;
+ len -= tlvlen + sizeof(*tlv);
+ assoc += tlvlen + sizeof(*tlv);
+ tlv = (struct amp_tlv *) assoc;
+ } else
+ break;
}
}
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-06 6:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 8:15 [PATCH 0/3 BlueZ] hcidump: fix buffer overflow Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 1/3 BlueZ] hcidump: fixed AMP Assoc dump heap-over-flow Cho, Yu-Chen
2018-11-02 10:54 ` Luiz Augusto von Dentz
2018-11-06 6:57 ` Cho, Yu-Chen
2018-10-31 8:15 ` [PATCH 2/3 BlueZ] hcidump:fixed hci frame dump stack-buffer-overflow Cho, Yu-Chen
2018-11-02 11:11 ` Luiz Augusto von Dentz
2018-10-31 8:15 ` [PATCH 3/3 BlueZ] hcidump: Fix set_ext_ctrl() global buffer overflow Cho, Yu-Chen
2018-11-02 11:12 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.