CVE-2022-20410 - 蓝牙 - 整数溢出->OOB Read

2023.09.17

patch

diff --git a/system/stack/avrc/avrc_pars_ct.cc b/system/stack/avrc/avrc_pars_ct.cc
index 12aee4c..a571042 100644
--- a/system/stack/avrc/avrc_pars_ct.cc
+++ b/system/stack/avrc/avrc_pars_ct.cc
@@ -237,7 +237,7 @@
   }
   BE_STREAM_TO_UINT8(pdu, p);
   uint16_t pkt_len;
-  int min_len = 0;
+  uint16_t min_len = 0;
   /* read the entire packet len */
   BE_STREAM_TO_UINT16(pkt_len, p);
 
@@ -380,8 +380,14 @@
               /* Parse the name now */
               BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
               BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
+              if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
+                  min_len) {
+                // Check for overflow
+                android_errorWriteLog(0x534e4554, "205570663");
+              }
+              if (pkt_len - min_len < attr_entry->name.str_len)
+                goto browse_length_error;
               min_len += attr_entry->name.str_len;
-              if (pkt_len < min_len) goto browse_length_error;
               attr_entry->name.p_str = (uint8_t*)osi_malloc(
                   attr_entry->name.str_len * sizeof(uint8_t));
               BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str,
@@ -444,8 +450,14 @@
         BE_STREAM_TO_UINT32(attr_entry->attr_id, p);
         BE_STREAM_TO_UINT16(attr_entry->name.charset_id, p);
         BE_STREAM_TO_UINT16(attr_entry->name.str_len, p);
+        if (static_cast<uint16_t>(min_len + attr_entry->name.str_len) <
+            min_len) {
+          // Check for overflow
+          android_errorWriteLog(0x534e4554, "205570663");
+        }
+        if (pkt_len - min_len < attr_entry->name.str_len)
+          goto browse_length_error;
         min_len += attr_entry->name.str_len;
-        if (pkt_len < min_len) goto browse_length_error;
         attr_entry->name.p_str =
             (uint8_t*)osi_malloc(attr_entry->name.str_len * sizeof(uint8_t));
         BE_STREAM_TO_ARRAY(p, attr_entry->name.p_str, attr_entry->name.str_len);
@@ -815,8 +827,12 @@
           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) {
+          if (static_cast<uint16_t>(min_len + p_attrs[i].name.str_len) <
+              min_len) {
+            // Check for overflow
+            android_errorWriteLog(0x534e4554, "205570663");
+          }
+          if (len - min_len < p_attrs[i].name.str_len) {
             for (int j = 0; j < i; j++) {
               osi_free(p_attrs[j].name.p_str);
             }
@@ -824,6 +840,7 @@
             p_result->get_attrs.num_attrs = 0;
             goto length_error;
           }
+          min_len += p_attrs[i].name.str_len;
           if (p_attrs[i].name.str_len > 0) {
             p_attrs[i].name.p_str =
                 (uint8_t*)osi_calloc(p_attrs[i].name.str_len);

分析

下图为hanzinuo大神的PPT，看这一页很容易理解：

• min_len是一个uint16_t类型的变量，初始值为0，然后+8
• 从packet解析得到变量attr_entyr->name.str_len，该变量的取值范围是0～UINT16_MAX
• 将attr_entyr->name.str_len的值添加到min_len，因为这里没有做判断，所以如果attr_entyr->name.str_len很大，那么就可能导致整数溢出
• 下一行的if判断中，是想要判断packet是否>=min_len，如果满足就继续往下走。而前面如果发生整数溢出，那么min_len变得很小，就会绕过这个检查
• 接着，malloc分配的chunk的大小是attr_entyr->name.str_len，会大于packet的实际长度，就会导致OOB Read

再来看看patch，好像和ppt里有点对不上？
不过单看patch也能看明白：在三处BE_STREAM_TO_UINT16(p_attrs[i].name.str_len, p);后面都添加了长度检查，这个不难理解。
我唯一不理解的就是第一处patch把一个min_len从int改成了uint_16？这是为啥？改错了？我看了一下main分支的代码，所有min_len都改成了uint32_t。