From 5d60f28e896002f324826c2c19369c066483e112 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 3 Sep 2021 14:39:19 -0400 Subject: [PATCH] Disallow VLAs in Matter code by adding -Wvla to our strict warnings config. We suppress the warning in two places, both tracked by existing issues. --- build/config/compiler/BUILD.gn | 1 + .../templates/app/CHIPClientCallbacks-src.zapt | 4 ++++ src/crypto/tests/CHIPCryptoPALTest.cpp | 18 ++++++++++++------ .../CHIP/CHIPOperationalCredentialsDelegate.mm | 4 ++-- .../CHIP/templates/CHIPClustersObjc-src.zapt | 11 +++++------ .../ManualSetupPayloadGenerator.cpp | 5 ++--- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn index f4da368abfb2f7..a7e1fda362b0c1 100644 --- 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 index 19767f5ae71752..29d6ffa23fe2de 100644 --- 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 index 0200e55d20f1f6..1a2666cc735bd8 100644 --- 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 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 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 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 index 8da338b39e7158..f56795af9052bf 100644 --- 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/CHIPClustersObjc-src.zapt b/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt index 5d1b1b5bf28522..2892704f336468 100644 --- a/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt +++ b/src/darwin/Framework/CHIP/templates/CHIPClustersObjc-src.zapt @@ -88,11 +88,11 @@ public: CHIP{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}AttributeCallbackBridge * callback = reinterpret_cast(context); if (callback && callback->mQueue) { - id values[count]; + id array = [NSMutableArray arrayWithCapacity:count]; for (uint16_t i = 0; i < count; i++) { {{#if isStruct}} - values[i] = [[NSDictionary alloc] initWithObjectsAndKeys: + array[i] = [[NSDictionary alloc] initWithObjectsAndKeys: {{#chip_attribute_list_entryTypes}} {{#if (isString type)}} {{#if (isOctetString type)}} @@ -109,17 +109,16 @@ public: {{else}} {{#if (isString type)}} {{#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}} - 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]; {{/if}} {{else}} - values[i] = [NSNumber numberWith{{asObjectiveCNumberType label type false}}:entries[i]]; + array[i] = [NSNumber numberWith{{asObjectiveCNumberType label type false}}:entries[i]]; {{/if}} {{/if}} } - id array = [NSArray arrayWithObjects:values count:count]; dispatch_async(callback->mQueue, ^{ callback->mHandler(nil, @{ @"value": array }); callback->Cancel(); diff --git a/src/setup_payload/ManualSetupPayloadGenerator.cpp b/src/setup_payload/ManualSetupPayloadGenerator.cpp index 2d53b117c6e9d1..7bf887eed7c313 100644 --- 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) {