Disallow VLAs in Matter code by adding -Wvla to our strict warnings c…

…onfig. We suppress the warning in two places, both tracked by existing issues.
project-chip · Sep 8, 2021 · 466ffc1 · 466ffc1
1 parent 6461373
commit 466ffc1
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 20 deletions.
diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn
@@ -200,6 +200,7 @@ config("strict_warnings") {
     "-Wextra",
     "-Wshadow",
     "-Wunreachable-code",
+    "-Wvla",
   ]
 
   cflags_cc = [ "-Wnon-virtual-dtor" ]

diff --git a/src/app/zap-templates/templates/app/CHIPClientCallbacks-src.zapt b/src/app/zap-templates/templates/app/CHIPClientCallbacks-src.zapt
@@ -353,7 +353,11 @@ void {{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}ListAttribu
 
     CHECK_MESSAGE_LENGTH_VOID(2);
     uint16_t count = Encoding::LittleEndian::Read16(message);
+    {{! TODO: This VLA is not OK.  See https://github.com/project-chip/connectedhomeip/issues/8558 }}
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wvla"
     {{chipType}} data[count];
+#pragma GCC diagnostic pop
     for (size_t i = 0; i < count; i++)
     {
         {{#if isStruct}}

diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp
@@ -555,9 +555,11 @@ static void TestAES_CCM_128DecryptInvalidCipherText(nlTestSuite * inSuite, void
         if (vector->pt_len > 0)
         {
             numOfTestsRan++;
-            uint8_t out_pt[vector->pt_len];
+            Platform::ScopedMemoryBuffer<uint8_t> out_pt;
+            out_pt.Alloc(vector->pt_len);
+            NL_TEST_ASSERT(inSuite, out_pt);
             CHIP_ERROR err = AES_CCM_decrypt(vector->ct, 0, vector->aad, vector->aad_len, vector->tag, vector->tag_len, vector->key,
-                                             vector->key_len, vector->iv, vector->iv_len, out_pt);
+                                             vector->key_len, vector->iv, vector->iv_len, out_pt.Get());
             NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
             break;
         }
@@ -575,9 +577,11 @@ static void TestAES_CCM_128DecryptInvalidKey(nlTestSuite * inSuite, void * inCon
         if (vector->pt_len > 0)
         {
             numOfTestsRan++;
-            uint8_t out_pt[vector->pt_len];
+            Platform::ScopedMemoryBuffer<uint8_t> out_pt;
+            out_pt.Alloc(vector->pt_len);
+            NL_TEST_ASSERT(inSuite, out_pt);
             CHIP_ERROR err = AES_CCM_decrypt(vector->ct, vector->ct_len, vector->aad, vector->aad_len, vector->tag, vector->tag_len,
-                                             nullptr, 0, vector->iv, vector->iv_len, out_pt);
+                                             nullptr, 0, vector->iv, vector->iv_len, out_pt.Get());
             NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
             break;
         }
@@ -595,9 +599,11 @@ static void TestAES_CCM_128DecryptInvalidIVLen(nlTestSuite * inSuite, void * inC
         if (vector->pt_len > 0)
         {
             numOfTestsRan++;
-            uint8_t out_pt[vector->pt_len];
+            Platform::ScopedMemoryBuffer<uint8_t> out_pt;
+            out_pt.Alloc(vector->pt_len);
+            NL_TEST_ASSERT(inSuite, out_pt);
             CHIP_ERROR err = AES_CCM_decrypt(vector->ct, vector->ct_len, vector->aad, vector->aad_len, vector->tag, vector->tag_len,
-                                             vector->key, vector->key_len, vector->iv, 0, out_pt);
+                                             vector->key, vector->key_len, vector->iv, 0, out_pt.Get());
             NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
             break;
         }

diff --git a/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm
@@ -84,8 +84,8 @@ static BOOL isRunningTests(void)
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    uint16_t idStringLen = 16;
-    char issuerIdString[idStringLen];
+    char issuerIdString[16];
+    uint16_t idStringLen = sizeof(issuerIdString);
     if (CHIP_NO_ERROR != storage->SyncGetKeyValue(CHIP_COMMISSIONER_CA_ISSUER_ID, issuerIdString, idStringLen)) {
         mIssuerId = arc4random();
         CHIP_LOG_ERROR("Assigned %d certificate issuer ID to the commissioner", mIssuerId);

diff --git a/src/darwin/Framework/CHIP/templates/partials/CHIPCallbackBridge.zapt b/src/darwin/Framework/CHIP/templates/partials/CHIPCallbackBridge.zapt
@@ -45,11 +45,11 @@ void CHIP{{@partial-block}}Bridge::OnSuccessFn(void * context
       {{/chip_cluster_response_arguments}}
     });
     {{else if (isStrEqual partial-type "List")}}
-    id values[count];
+    id array = [NSMutableArray arrayWithCapacity:count];
     for (uint16_t i = 0; i < count; i++)
     {
         {{#if isStruct}}
-        values[i] = @{
+        array[i] = @{
           {{#chip_attribute_list_entryTypes}}
           {{#if (isOctetString type)}}
           @"{{name}}" : [NSData dataWithBytes:entries[i].{{name}}.data() length:entries[i].{{name}}.size()],
@@ -61,15 +61,15 @@ void CHIP{{@partial-block}}Bridge::OnSuccessFn(void * context
           {{/chip_attribute_list_entryTypes}}
         };
         {{else if (isOctetString type)}}
-        values[i] = [NSData dataWithBytes:entries[i].data() length:entries[i].size()];
+        array[i] = [NSData dataWithBytes:entries[i].data() length:entries[i].size()];
         {{else if (isCharString type)}}
-        values[i] = [[NSString alloc] initWithBytes:entries[i]+{{asReadTypeLength type}} length:emberAf{{asReadType type}}Length(entries[i]) encoding:NSUTF8StringEncoding];
+        array[i] = [[NSString alloc] initWithBytes:entries[i]+{{asReadTypeLength type}} length:emberAf{{asReadType type}}Length(entries[i]) encoding:NSUTF8StringEncoding];
         {{else}}
-        values[i] = [NSNumber numberWith{{asObjectiveCNumberType label type false}}:entries[i]];
+        array[i] = [NSNumber numberWith{{asObjectiveCNumberType label type false}}:entries[i]];
         {{/if}}
     }
 
-    DispatchSuccess(context, @{ @"value": [NSArray arrayWithObjects:values count:count] });
+    DispatchSuccess(context, @{ @"value": array });
     {{else if (isOctetString partial-type)}}
     DispatchSuccess(context, @{ @"value": [NSData dataWithBytes: value.data() length: value.size()] });
     {{else if (isCharString partial-type)}}

diff --git a/src/platform/Linux/KeyValueStoreManagerImpl.cpp b/src/platform/Linux/KeyValueStoreManagerImpl.cpp
@@ -57,15 +57,16 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t
         return err;
     }
 
-    uint8_t buf[read_size];
-    ReturnErrorOnFailure(mStorage.ReadValueBin(key, buf, read_size, read_size));
+    Platform::ScopedMemoryBuffer<uint8_t> buf;
+    VerifyOrReturnError(buf.Alloc(read_size), CHIP_ERROR_NO_MEMORY);
+    ReturnErrorOnFailure(mStorage.ReadValueBin(key, buf.Get(), read_size, read_size));
 
     size_t copy_size = std::min(value_size, read_size - offset_bytes);
     if (read_bytes_size != nullptr)
     {
         *read_bytes_size = copy_size;
     }
-    ::memcpy(value, buf + offset_bytes, copy_size);
+    ::memcpy(value, buf.Get() + offset_bytes, copy_size);
 
     return CHIP_NO_ERROR;
 }

diff --git a/src/setup_payload/ManualSetupPayloadGenerator.cpp b/src/setup_payload/ManualSetupPayloadGenerator.cpp
@@ -88,10 +88,11 @@ static uint32_t chunk3PayloadRepresentation(const SetupPayload & payload)
 }
 
 // TODO: issue #3663 - Unbounded stack in src/setup_payload
-#if !defined(__clang__)
 #pragma GCC diagnostic push
+#if !defined(__clang__)
 #pragma GCC diagnostic ignored "-Wstack-usage="
 #endif
+#pragma GCC diagnostic ignored "-Wvla"
 
 static std::string decimalStringWithPadding(uint32_t number, int minLength)
 {
@@ -100,9 +101,7 @@ static std::string decimalStringWithPadding(uint32_t number, int minLength)
     return std::string(buf);
 }
 
-#if !defined(__clang__)
 #pragma GCC diagnostic pop
-#endif
 
 CHIP_ERROR ManualSetupPayloadGenerator::payloadDecimalStringRepresentation(std::string & outDecimalString)
 {