ad_parser: validate data element length fields in ad_iterator_has_more

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - L2CAP: emit L2CAP_EVENT_ERTM_BUFFER_RELEASED if ERTM buffer not needed/used anymore
 - L2CAP: add fcs_option to ERTM config l2cap_ertm_config_t
 - HCI: validate advertisement data length field when generating GAP_EVENT_ADVERTISING_REPORT
+- ad_parser: validate data element length fields in ad_iterator_has_more
 
 ## Changes December 2018
 

src/ad_parser.c

@@ -64,17 +64,23 @@ void ad_iterator_init(ad_context_t *context, uint8_t ad_len, const uint8_t * ad_
 }
 
 int  ad_iterator_has_more(const ad_context_t * context){
-    return context->offset < context->length;
+    // assert chunk_len and chunk_type are withing buffer
+    if ((context->offset+1) >= context->length) return 0;
+
+    // assert chunk_len > 0
+    int chunk_len = context->data[context->offset];
+    if (chunk_len == 0)                                    return 0;
+
+    // assert complete chunk fits into buffer
+    if (context->offset + 1 + chunk_len > context->length) return 0;
+
+    return 1;
 }
 
+// pre: ad_iterator_has_more() == 1
 void ad_iterator_next(ad_context_t * context){
     int chunk_len = context->data[context->offset];
-    int new_offset = context->offset + 1 + chunk_len;
-    // avoid uint8_t overrun
-    if (new_offset > 0xff){
-        new_offset = 0xff;
-    }
-    context->offset = new_offset;
+    context->offset += 1 + chunk_len;
 }
 
 uint8_t   ad_iterator_get_data_len(const ad_context_t * context){

test/ble_client/advertising_data_parser.c

@@ -185,11 +185,24 @@ TEST(ADParser, TestDataParsing){
     }
 }
 
-
 TEST(ADParser, TestAdvertisementEventMultipleReports){
     le_handle_advertisement_report(adv_multi_packet, sizeof(adv_multi_packet));
 }
 
+TEST(ADParser, TestMalformed){
+    ad_context_t context;
+
+    // len = 0xff, but only one byte type
+    uint8_t data[] = { 0xff, 0x01 };
+    ad_iterator_init(&context, sizeof(data), data);
+    CHECK_EQUAL(ad_iterator_has_more(&context), 0);
+
+    // len = 0x01, but not type
+    uint8_t data2[] = { 0x00, 0x01 };
+    ad_iterator_init(&context, sizeof(data2), data2);
+    CHECK_EQUAL(ad_iterator_has_more(&context), 0);
+}
+
 int main (int argc, const char * argv[]){
     return CommandLineTestRunner::RunAllTests(argc, argv);
-}
+}