OSDN Git Service

DO NOT MERGE - Check AVRCP data length when parsing inside avrc_ctrl_pars_vendor_rsp()
authorPavlin Radoslavov <pavlin@google.com>
Thu, 6 Sep 2018 22:41:27 +0000 (15:41 -0700)
committerRyan Longair <rlongair@google.com>
Fri, 7 Sep 2018 17:30:18 +0000 (10:30 -0700)
Bug: 111450417
Test: PoC test program
Change-Id: Idd619e52dc7a2944d0d08af824505580e299c163
(cherry picked from commit 2692408d05bf16738284b61833649cee5d2a2233)
(cherry picked from commit b4cf8416bfd7922be02a246f50059c1309d6afc4)

stack/avrc/avrc_pars_ct.c

index dbbd8f1..71e0e34 100644 (file)
@@ -30,6 +30,8 @@
 
 #if (AVRC_METADATA_INCLUDED == TRUE)
 
+#define MIN(x, y) ((x) < (y) ? (x) : (y))
+
 /*******************************************************************************
 **
 ** Function         avrc_pars_vendor_rsp
@@ -141,25 +143,37 @@ static tAVRC_STS avrc_pars_vendor_rsp(tAVRC_MSG_VENDOR *p_msg, tAVRC_RESPONSE *p
     return status;
 }
 
-void avrc_parse_notification_rsp (UINT8 *p_stream, tAVRC_REG_NOTIF_RSP *p_rsp)
+tAVRC_STS avrc_parse_notification_rsp (UINT8 *p_stream, UINT16 len,
+                                       tAVRC_REG_NOTIF_RSP *p_rsp)
 {
+    UINT16 min_len = 1;
+
+    if (len < min_len) goto length_error;
     BE_STREAM_TO_UINT8(p_rsp->event_id, p_stream);
     switch (p_rsp->event_id)
     {
         case AVRC_EVT_PLAY_STATUS_CHANGE:
+            min_len += 1;
+            if (len < min_len) goto length_error;
             BE_STREAM_TO_UINT8(p_rsp->param.play_status, p_stream);
             break;
 
         case AVRC_EVT_TRACK_CHANGE:
+            min_len += 8;
+            if (len < min_len) goto length_error;
             BE_STREAM_TO_ARRAY(p_stream, p_rsp->param.track, 8);
             break;
 
         case AVRC_EVT_APP_SETTING_CHANGE:
+            min_len += 1;
+            if (len < min_len) goto length_error;
             BE_STREAM_TO_UINT8(p_rsp->param.player_setting.num_attr, p_stream);
             if (p_rsp->param.player_setting.num_attr > AVRC_MAX_APP_SETTINGS) {
                 android_errorWriteLog(0x534e4554, "73782082");
                 p_rsp->param.player_setting.num_attr = AVRC_MAX_APP_SETTINGS;
             }
+            min_len += p_rsp->param.player_setting.num_attr * 2;
+            if (len < min_len) goto length_error;
             for (int index = 0; index < p_rsp->param.player_setting.num_attr; index++)
             {
                 BE_STREAM_TO_UINT8(p_rsp->param.player_setting.attr_id[index], p_stream);
@@ -187,6 +201,14 @@ void avrc_parse_notification_rsp (UINT8 *p_stream, tAVRC_REG_NOTIF_RSP *p_rsp)
         default:
             break;
     }
+
+    return AVRC_STS_NO_ERROR;
+
+length_error:
+    android_errorWriteLog(0x534e4554, "111450417");
+    AVRC_TRACE_WARNING("%s: invalid parameter length %d: must be at least %d",
+                       __func__, len, min_len);
+    return AVRC_STS_INTERNAL_ERR;
 }
 
 #if (AVRC_CTLR_INCLUDED == TRUE)
@@ -204,17 +226,33 @@ void avrc_parse_notification_rsp (UINT8 *p_stream, tAVRC_REG_NOTIF_RSP *p_rsp)
 static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
     tAVRC_MSG_VENDOR *p_msg, tAVRC_RESPONSE *p_result, UINT8* p_buf, UINT16* buf_len)
 {
+    if (p_msg->vendor_len < 4) {
+        android_errorWriteLog(0x534e4554, "111450417");
+        AVRC_TRACE_WARNING("%s: message length %d too short: must be at least 4",
+                           __func__, p_msg->vendor_len);
+        return AVRC_STS_INTERNAL_ERR;
+    }
+
     UINT8   *p = p_msg->p_vendor_data;
     BE_STREAM_TO_UINT8 (p_result->pdu, p);
     p++; /* skip the reserved/packe_type byte */
 
     UINT16  len;
+    UINT16  min_len = 0;
     BE_STREAM_TO_UINT16 (len, p);
-    AVRC_TRACE_DEBUG("%s ctype:0x%x pdu:0x%x, len:%d",
-                     __func__, p_msg->hdr.ctype, p_result->pdu, len);
+    AVRC_TRACE_DEBUG("%s ctype:0x%x pdu:0x%x, len:%d  vendor_len=0x%x", __func__,
+                     p_msg->hdr.ctype, p_result->pdu, len, p_msg->vendor_len);
+    if (p_msg->vendor_len < len + 4) {
+        android_errorWriteLog(0x534e4554, "111450417");
+        AVRC_TRACE_WARNING("%s: message length %d too short: must be at least %d",
+                           __func__, p_msg->vendor_len, len + 4);
+        return AVRC_STS_INTERNAL_ERR;
+    }
     /* Todo: Issue in handling reject, check */
     if (p_msg->hdr.ctype == AVRC_RSP_REJ)
     {
+        min_len += 1;
+        if (len < min_len) goto length_error;
         p_result->rsp.status = *p;
         return p_result->rsp.status;
     }
@@ -226,8 +264,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
     /* case AVRC_PDU_ABORT_CONTINUATION_RSP:   0x41 */
 
     case AVRC_PDU_REGISTER_NOTIFICATION:
-        avrc_parse_notification_rsp(p, &p_result->reg_notif);
-        break;
+        return avrc_parse_notification_rsp(p, len, &p_result->reg_notif);
 
     case AVRC_PDU_GET_CAPABILITIES:
         if (len == 0)
@@ -236,12 +273,16 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->get_caps.capability_id = 0;
             break;
         }
+        min_len += 2;
+        if (len < min_len) goto length_error;
         BE_STREAM_TO_UINT8(p_result->get_caps.capability_id, p);
         BE_STREAM_TO_UINT8(p_result->get_caps.count, p);
         AVRC_TRACE_DEBUG("%s cap id = %d, cap_count = %d ",
                          __func__, p_result->get_caps.capability_id, p_result->get_caps.count);
         if (p_result->get_caps.capability_id == AVRC_CAP_COMPANY_ID)
         {
+            min_len += MIN(p_result->get_caps.count, AVRC_CAP_MAX_NUM_COMP_ID) * 3;
+            if (len < min_len) goto length_error;
             for(int xx = 0; ((xx < p_result->get_caps.count) && (xx < AVRC_CAP_MAX_NUM_COMP_ID));
                 xx++)
             {
@@ -250,6 +291,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         }
         else if (p_result->get_caps.capability_id == AVRC_CAP_EVENTS_SUPPORTED)
         {
+            min_len += MIN(p_result->get_caps.count, AVRC_CAP_MAX_NUM_EVT_ID);
+            if (len < min_len) goto length_error;
             for(int xx = 0; ((xx < p_result->get_caps.count) && (xx < AVRC_CAP_MAX_NUM_EVT_ID));
                 xx++)
             {
@@ -264,6 +307,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->list_app_attr.num_attr = 0;
             break;
         }
+        min_len += 1;
         BE_STREAM_TO_UINT8(p_result->list_app_attr.num_attr, p);
 
         if (p_result->list_app_attr.num_attr > AVRC_MAX_APP_ATTR_SIZE) {
@@ -272,6 +316,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         }
 
         AVRC_TRACE_DEBUG("%s attr count = %d ", __func__, p_result->list_app_attr.num_attr);
+        min_len += p_result->list_app_attr.num_attr;
+        if (len < min_len) goto length_error;
         for(int xx = 0; xx < p_result->list_app_attr.num_attr; xx++)
         {
             BE_STREAM_TO_UINT8(p_result->list_app_attr.attrs[xx], p);
@@ -284,6 +330,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->list_app_values.num_val = 0;
             break;
         }
+        min_len += 1;
         BE_STREAM_TO_UINT8(p_result->list_app_values.num_val, p);
         if (p_result->list_app_values.num_val > AVRC_MAX_APP_ATTR_SIZE) {
             android_errorWriteLog(0x534e4554, "78526423");
@@ -291,6 +338,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         }
 
         AVRC_TRACE_DEBUG("%s value count = %d ", __func__, p_result->list_app_values.num_val);
+        min_len += p_result->list_app_values.num_val;
+        if (len < min_len) goto length_error;
         for(int xx = 0; xx < p_result->list_app_values.num_val; xx++)
         {
             BE_STREAM_TO_UINT8(p_result->list_app_values.vals[xx], p);
@@ -304,9 +353,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->get_cur_app_val.num_val = 0;
             break;
         }
+        min_len += 1;
         BE_STREAM_TO_UINT8(p_result->get_cur_app_val.num_val, p);
-        tAVRC_APP_SETTING *app_sett =
-            (tAVRC_APP_SETTING*)osi_malloc(p_result->get_cur_app_val.num_val*sizeof(tAVRC_APP_SETTING));
         AVRC_TRACE_DEBUG("%s attr count = %d ", __func__, p_result->get_cur_app_val.num_val);
 
 
@@ -315,6 +363,13 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->get_cur_app_val.num_val = AVRC_MAX_APP_ATTR_SIZE;
         }
 
+        min_len += p_result->get_cur_app_val.num_val * 2;
+        if (len < min_len) {
+            p_result->get_cur_app_val.num_val = 0;
+            goto length_error;
+        }
+        tAVRC_APP_SETTING *app_sett =
+            (tAVRC_APP_SETTING*)osi_calloc(p_result->get_cur_app_val.num_val*sizeof(tAVRC_APP_SETTING));
         for (int xx = 0; xx < p_result->get_cur_app_val.num_val; xx++)
         {
             BE_STREAM_TO_UINT8(app_sett[xx].attr_id, p);
@@ -333,6 +388,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->get_app_attr_txt.num_attr = 0;
             break;
         }
+        min_len += 1;
         BE_STREAM_TO_UINT8(num_attrs, p);
         if (num_attrs > AVRC_MAX_APP_ATTR_SIZE) {
           num_attrs = AVRC_MAX_APP_ATTR_SIZE;
@@ -340,15 +396,33 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         AVRC_TRACE_DEBUG("%s attr count = %d ", __func__, p_result->get_app_attr_txt.num_attr);
         p_result->get_app_attr_txt.num_attr = num_attrs;
         p_result->get_app_attr_txt.p_attrs = (tAVRC_APP_SETTING_TEXT*)
-            osi_malloc(num_attrs * sizeof(tAVRC_APP_SETTING_TEXT));
+            osi_calloc(num_attrs * sizeof(tAVRC_APP_SETTING_TEXT));
         for (int xx = 0; xx < num_attrs; xx++)
         {
+            min_len += 4;
+            if (len < min_len) {
+                for (int j = 0; j < xx; j++) {
+                    osi_free(p_result->get_app_attr_txt.p_attrs[j].p_str);
+                }
+                osi_free_and_reset((void**)&p_result->get_app_attr_txt.p_attrs);
+                p_result->get_app_attr_txt.num_attr = 0;
+                goto length_error;
+            }
             BE_STREAM_TO_UINT8(p_result->get_app_attr_txt.p_attrs[xx].attr_id, p);
             BE_STREAM_TO_UINT16(p_result->get_app_attr_txt.p_attrs[xx].charset_id, p);
             BE_STREAM_TO_UINT8(p_result->get_app_attr_txt.p_attrs[xx].str_len, p);
+            min_len += p_result->get_app_attr_txt.p_attrs[xx].str_len;
+            if (len < min_len) {
+                for (int j = 0; j < xx; j++) {
+                    osi_free(p_result->get_app_attr_txt.p_attrs[j].p_str);
+                }
+                osi_free_and_reset((void**)&p_result->get_app_attr_txt.p_attrs);
+                p_result->get_app_attr_txt.num_attr = 0;
+                goto length_error;
+            }
             if (p_result->get_app_attr_txt.p_attrs[xx].str_len != 0)
             {
-                UINT8 *p_str = (UINT8 *)osi_malloc(p_result->get_app_attr_txt.p_attrs[xx].str_len);
+                UINT8 *p_str = (UINT8 *)osi_calloc(p_result->get_app_attr_txt.p_attrs[xx].str_len);
                 BE_STREAM_TO_ARRAY(p, p_str, p_result->get_app_attr_txt.p_attrs[xx].str_len);
                 p_result->get_app_attr_txt.p_attrs[xx].p_str = p_str;
             } else {
@@ -367,6 +441,7 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->get_app_val_txt.num_attr = 0;
             break;
         }
+        min_len += 1;
         BE_STREAM_TO_UINT8(num_vals, p);
         if (num_vals > AVRC_MAX_APP_ATTR_SIZE) {
           num_vals = AVRC_MAX_APP_ATTR_SIZE;
@@ -375,13 +450,31 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         AVRC_TRACE_DEBUG("%s value count = %d ", __func__, p_result->get_app_val_txt.num_attr);
 
         p_result->get_app_val_txt.p_attrs = (tAVRC_APP_SETTING_TEXT *)
-            osi_malloc(num_vals * sizeof(tAVRC_APP_SETTING_TEXT));
+            osi_calloc(num_vals * sizeof(tAVRC_APP_SETTING_TEXT));
         for (int i = 0; i < num_vals; i++) {
+            min_len += 4;
+            if (len < min_len) {
+                for (int j = 0; j < i; j++) {
+                    osi_free(p_result->get_app_val_txt.p_attrs[j].p_str);
+                }
+                osi_free_and_reset((void**)&p_result->get_app_val_txt.p_attrs);
+                p_result->get_app_val_txt.num_attr = 0;
+                goto length_error;
+            }
             BE_STREAM_TO_UINT8(p_result->get_app_val_txt.p_attrs[i].attr_id, p);
             BE_STREAM_TO_UINT16(p_result->get_app_val_txt.p_attrs[i].charset_id, p);
             BE_STREAM_TO_UINT8(p_result->get_app_val_txt.p_attrs[i].str_len, p);
+            min_len += p_result->get_app_val_txt.p_attrs[i].str_len;
+            if (len < min_len) {
+                for (int j = 0; j < i; j++) {
+                    osi_free(p_result->get_app_val_txt.p_attrs[j].p_str);
+                }
+                osi_free_and_reset((void**)&p_result->get_app_val_txt.p_attrs);
+                p_result->get_app_val_txt.num_attr = 0;
+                goto length_error;
+            }
             if (p_result->get_app_val_txt.p_attrs[i].str_len != 0) {
-                UINT8 *p_str = (UINT8 *)osi_malloc(p_result->get_app_val_txt.p_attrs[i].str_len);
+                UINT8 *p_str = (UINT8 *)osi_calloc(p_result->get_app_val_txt.p_attrs[i].str_len);
                 BE_STREAM_TO_ARRAY(p, p_str, p_result->get_app_val_txt.p_attrs[i].str_len);
                 p_result->get_app_val_txt.p_attrs[i].p_str = p_str;
             } else {
@@ -404,19 +497,40 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
             p_result->get_elem_attrs.num_attr = 0;
             break;
         }
+        min_len += 1;
         BE_STREAM_TO_UINT8(num_attrs, p);
         p_result->get_elem_attrs.num_attr = num_attrs;
         if (num_attrs)
         {
             tAVRC_ATTR_ENTRY *p_attrs =
-                (tAVRC_ATTR_ENTRY*)osi_malloc(num_attrs * sizeof(tAVRC_ATTR_ENTRY));
+                (tAVRC_ATTR_ENTRY*)osi_calloc(num_attrs * sizeof(tAVRC_ATTR_ENTRY));
             for (int i = 0; i < num_attrs; i++) {
+                min_len += 8;
+                if (len < min_len) {
+                    for (int j = 0; j < i; j++) {
+                        osi_free(p_attrs[j].name.p_str);
+                    }
+                    osi_free(p_attrs);
+                    p_result->get_elem_attrs.num_attr = 0;
+                    goto length_error;
+                }
                 BE_STREAM_TO_UINT32(p_attrs[i].attr_id, p);
                 BE_STREAM_TO_UINT16(p_attrs[i].name.charset_id, p);
                 BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);
+                min_len += p_attrs[i].name.str_len;
+                if (len < min_len) {
+                    for (int j = 0; j < i; j++) {
+                        osi_free(p_attrs[j].name.p_str);
+                    }
+                    osi_free(p_attrs);
+                    p_result->get_elem_attrs.num_attr = 0;
+                    goto length_error;
+                }
                 if (p_attrs[i].name.str_len > 0) {
-                    p_attrs[i].name.p_str = (UINT8 *)osi_malloc(p_attrs[i].name.str_len);
+                    p_attrs[i].name.p_str = (UINT8 *)osi_calloc(p_attrs[i].name.str_len);
                     BE_STREAM_TO_ARRAY(p, p_attrs[i].name.p_str, p_attrs[i].name.str_len);
+                } else {
+                    p_attrs[i].name.p_str = NULL;
                 }
             }
             p_result->get_elem_attrs.p_attrs = p_attrs;
@@ -429,6 +543,8 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         {
             break;
         }
+        min_len += 9;
+        if (len < min_len) goto length_error;
         BE_STREAM_TO_UINT32(p_result->get_play_status.song_len, p);
         BE_STREAM_TO_UINT32(p_result->get_play_status.song_pos, p);
         BE_STREAM_TO_UINT8(p_result->get_play_status.status, p);
@@ -438,6 +554,12 @@ static tAVRC_STS avrc_ctrl_pars_vendor_rsp(
         return AVRC_STS_BAD_CMD;
     }
     return AVRC_STS_NO_ERROR;
+
+length_error:
+    android_errorWriteLog(0x534e4554, "111450417");
+    AVRC_TRACE_WARNING("%s: invalid parameter length %d: must be at least %d",
+                       __func__, len, min_len);
+    return AVRC_STS_INTERNAL_ERR;
 }
 
 /*******************************************************************************