From 08a69d36726b6345df6e64892cadd5ab5d5ca2a6 Mon Sep 17 00:00:00 2001 From: "Cho, Yu-Chen" Date: Tue, 19 Mar 2019 15:54:09 +0800 Subject: [PATCH BlueZ] hcidump: Add assoc dump function assoc date length check amp_assoc_dump() didn't check the length of amp assoc struct. If there is wrong length size of assoc date, amp_assoc_dump() and amp_dump_chanlist() will read over the size(heap-buffer-overflow). use t_len to save the length avoid use the wrong size of date. --- tools/parser/amp.c | 35 +++++++++++++++++++++++++++-------- tools/parser/hci.c | 4 ++-- tools/parser/l2cap.c | 6 ++++-- tools/parser/parser.h | 2 +- 4 files changed, 34 insertions(+), 13 deletions(-) Index: bluez-5.65/tools/parser/amp.c =================================================================== --- bluez-5.65.orig/tools/parser/amp.c +++ bluez-5.65/tools/parser/amp.c @@ -15,7 +15,8 @@ #include "parser.h" #include "lib/amp.h" -static void amp_dump_chanlist(int level, struct amp_tlv *tlv, char *prefix) +static void amp_dump_chanlist(int level, struct amp_tlv *tlv, + uint16_t t_len, char *prefix) { struct amp_chan_list *chan_list = (void *) tlv->val; struct amp_country_triplet *triplet; @@ -25,6 +26,12 @@ static void amp_dump_chanlist(int level, printf("%s (number of triplets %d)\n", prefix, num); + if (btohs(tlv->len) > t_len) { + p_indent(level+1, 0); + printf("Wrong number of triplets\n"); + num = (t_len - sizeof(*chan_list)) / sizeof(*triplet); + } + p_indent(level+2, 0); printf("Country code: %c%c%c\n", chan_list->country_code[0], @@ -55,7 +62,7 @@ static void amp_dump_chanlist(int level, } } -void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len) +void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len, uint16_t t_len) { struct amp_tlv *tlv = (void *) assoc; @@ -63,6 +70,14 @@ void amp_assoc_dump(int level, uint8_t * printf("Assoc data [len %d]:\n", len); while (len > sizeof(*tlv)) { + if (btohs(tlv->len) > (t_len - sizeof(struct amp_tlv))) { + p_indent(level+1, 0); + printf("Assoc data get error size\n"); + t_len -= sizeof(struct amp_tlv); + } else { + t_len -= sizeof(struct amp_tlv) + btohs(tlv->len); + } + uint16_t tlvlen = btohs(tlv->len); struct amp_pal_ver *ver; @@ -78,11 +93,13 @@ void amp_assoc_dump(int level, uint8_t * break; case A2MP_PREF_CHANLIST_TYPE: - amp_dump_chanlist(level, tlv, "Preferred Chan List"); + amp_dump_chanlist(level, tlv, + t_len, "Preferred Chan List"); break; case A2MP_CONNECTED_CHAN: - amp_dump_chanlist(level, tlv, "Connected Chan List"); + amp_dump_chanlist(level, tlv, + t_len, "Connected Chan List"); break; case A2MP_PAL_CAP_TYPE: @@ -106,9 +123,11 @@ void amp_assoc_dump(int level, uint8_t * printf("Unrecognized type %d\n", tlv->type); break; } - - len -= tlvlen + sizeof(*tlv); - assoc += tlvlen + sizeof(*tlv); - tlv = (struct amp_tlv *) assoc; + if (btohs(tlv->len) <= t_len) { + len -= tlvlen + sizeof(*tlv); + assoc += tlvlen + sizeof(*tlv); + tlv = (struct amp_tlv *) assoc; + } else + len = 0; } } Index: bluez-5.65/tools/parser/hci.c =================================================================== --- bluez-5.65.orig/tools/parser/hci.c +++ bluez-5.65/tools/parser/hci.c @@ -1667,7 +1667,7 @@ static inline void write_remote_amp_asso printf("handle 0x%2.2x len_so_far %d remaining_len %d\n", cp->handle, cp->length_so_far, cp->remaining_length); - amp_assoc_dump(level + 1, cp->fragment, frm->len - 5); + amp_assoc_dump(level + 1, cp->fragment, frm->len - 5, frm->len - 5); } static inline void command_dump(int level, struct frame *frm) @@ -2650,7 +2650,7 @@ static inline void read_local_amp_assoc_ p_indent(level, frm); printf("Error: %s\n", status2str(rp->status)); } else { - amp_assoc_dump(level + 1, rp->fragment, len); + amp_assoc_dump(level + 1, rp->fragment, len, frm->len - 4); } } Index: bluez-5.65/tools/parser/l2cap.c =================================================================== --- bluez-5.65.orig/tools/parser/l2cap.c +++ bluez-5.65/tools/parser/l2cap.c @@ -1159,7 +1159,8 @@ static inline void a2mp_assoc_rsp(int le printf("Get AMP Assoc rsp: id %d status (%d) %s\n", h->id, h->status, a2mpstatus2str(h->status)); - amp_assoc_dump(level + 1, h->assoc_data, len - sizeof(*h)); + amp_assoc_dump(level + 1, h->assoc_data, + len - sizeof(*h), frm->len - sizeof(*h)); } static inline void a2mp_create_req(int level, struct frame *frm, uint16_t len) @@ -1168,7 +1169,8 @@ static inline void a2mp_create_req(int l printf("Create Physical Link req: local id %d remote id %d\n", h->local_id, h->remote_id); - amp_assoc_dump(level + 1, h->assoc_data, len - sizeof(*h)); + amp_assoc_dump(level + 1, h->assoc_data, + len - sizeof(*h), frm->len - sizeof(*h)); } static inline void a2mp_create_rsp(int level, struct frame *frm) Index: bluez-5.65/tools/parser/parser.h =================================================================== --- bluez-5.65.orig/tools/parser/parser.h +++ bluez-5.65/tools/parser/parser.h @@ -236,7 +236,7 @@ void ericsson_dump(int level, struct fra void csr_dump(int level, struct frame *frm); void bpa_dump(int level, struct frame *frm); -void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len); +void amp_assoc_dump(int level, uint8_t *assoc, uint16_t len, uint16_t t_len); static inline void parse(struct frame *frm) {