* [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
* 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@vger.kernel.org, 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
* [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
* [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
* 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@vger.kernel.org, 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
* [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 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@vger.kernel.org, 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).