Fixes review issue 25498 (#25502)

* add range-checking for credentialTypes and properly size credential array for silabs lock-app * Restyled by clang-format * code review comments, fix rs917 * Restyled by clang-format * Restyled by clang-format * fix nvm read/write for WX917 --------- Co-authored-by: Restyled.io <commits@restyled.io>
project-chip · Dec 15, 2023 · 1031530 · 1031530
1 parent 0465074
commit 1031530
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 25 deletions.
diff --git a/examples/lock-app/silabs/SiWx917/include/LockManager.h b/examples/lock-app/silabs/SiWx917/include/LockManager.h
@@ -175,6 +175,7 @@ class LockManager
 
     bool IsValidUserIndex(uint16_t userIndex);
     bool IsValidCredentialIndex(uint16_t credentialIndex, CredentialTypeEnum type);
+    bool IsValidCredentialType(CredentialTypeEnum type);
     bool IsValidWeekdayScheduleIndex(uint8_t scheduleIndex);
     bool IsValidYeardayScheduleIndex(uint8_t scheduleIndex);
     bool IsValidHolidayScheduleIndex(uint8_t scheduleIndex);

diff --git a/examples/lock-app/silabs/SiWx917/src/LockManager.cpp b/examples/lock-app/silabs/SiWx917/src/LockManager.cpp
@@ -113,6 +113,11 @@ bool LockManager::IsValidCredentialIndex(uint16_t credentialIndex, CredentialTyp
     return (credentialIndex < kMaxCredentialsPerUser);
 }
 
+bool LockManager::IsValidCredentialType(CredentialTypeEnum type)
+{
+    return (to_underlying(type) < kNumCredentialTypes);
+}
+
 bool LockManager::IsValidWeekdayScheduleIndex(uint8_t scheduleIndex)
 {
     return (scheduleIndex < kMaxWeekdaySchedulesPerUser);
@@ -134,9 +139,8 @@ bool LockManager::ReadConfigValues()
     SilabsConfig::ReadConfigValueBin(SilabsConfig::kConfigKey_LockUser, reinterpret_cast<uint8_t *>(&mLockUsers),
                                      sizeof(EmberAfPluginDoorLockUserInfo) * ArraySize(mLockUsers), outLen);
 
-    SilabsConfig::ReadConfigValueBin(
-        SilabsConfig::kConfigKey_Credential, reinterpret_cast<uint8_t *>(&mLockCredentials),
-        sizeof(EmberAfPluginDoorLockCredentialInfo) * LockParams.numberOfCredentialsPerUser * kNumCredentialTypes, outLen);
+    SilabsConfig::ReadConfigValueBin(SilabsConfig::kConfigKey_Credential, reinterpret_cast<uint8_t *>(&mLockCredentials),
+                                     sizeof(EmberAfPluginDoorLockCredentialInfo) * kMaxCredentials * kNumCredentialTypes, outLen);
 
     SilabsConfig::ReadConfigValueBin(SilabsConfig::kConfigKey_LockUserName, reinterpret_cast<uint8_t *>(mUserNames),
                                      sizeof(mUserNames), outLen);
@@ -379,9 +383,7 @@ bool LockManager::SetUser(chip::EndpointId endpointId, uint16_t userIndex, chip:
 
     for (size_t i = 0; i < totalCredentials; ++i)
     {
-        mCredentials[userIndex][i]                 = credentials[i];
-        mCredentials[userIndex][i].credentialType  = credentials[i].credentialType;
-        mCredentials[userIndex][i].credentialIndex = credentials[i].credentialIndex;
+        mCredentials[userIndex][i] = credentials[i];
     }
 
     userInStorage.credentials = chip::Span<const CredentialStruct>(mCredentials[userIndex], totalCredentials);
@@ -405,6 +407,8 @@ bool LockManager::GetCredential(chip::EndpointId endpointId, uint16_t credential
                                 EmberAfPluginDoorLockCredentialInfo & credential)
 {
 
+    VerifyOrReturnValue(IsValidCredentialType(credentialType), false);
+
     if (CredentialTypeEnum::kProgrammingPIN == credentialType)
     {
         VerifyOrReturnValue(IsValidCredentialIndex(credentialIndex, credentialType),
@@ -448,6 +452,8 @@ bool LockManager::SetCredential(chip::EndpointId endpointId, uint16_t credential
                                 const chip::ByteSpan & credentialData)
 {
 
+    VerifyOrReturnValue(IsValidCredentialType(credentialType), false);
+
     if (CredentialTypeEnum::kProgrammingPIN == credentialType)
     {
         VerifyOrReturnValue(IsValidCredentialIndex(credentialIndex, credentialType),
@@ -476,8 +482,7 @@ bool LockManager::SetCredential(chip::EndpointId endpointId, uint16_t credential
 
     // Save credential information in NVM flash
     SilabsConfig::WriteConfigValueBin(SilabsConfig::kConfigKey_Credential, reinterpret_cast<const uint8_t *>(&mLockCredentials),
-                                      sizeof(EmberAfPluginDoorLockCredentialInfo) * LockParams.numberOfCredentialsPerUser *
-                                          kNumCredentialTypes);
+                                      sizeof(EmberAfPluginDoorLockCredentialInfo) * kMaxCredentials * kNumCredentialTypes);
 
     SilabsConfig::WriteConfigValueBin(SilabsConfig::kConfigKey_CredentialData, reinterpret_cast<const uint8_t *>(&mCredentialData),
                                       sizeof(mCredentialData));
@@ -682,15 +687,15 @@ bool LockManager::setLockState(chip::EndpointId endpointId, DlLockState lockStat
     }
 
     // Check the PIN code
-    for (uint8_t i = 0; i < kMaxCredentialsPerUser; i++)
+    for (const auto & currentCredential : mLockCredentials[to_underlying(CredentialTypeEnum::kPin)])
     {
 
-        if (mLockCredentials[to_underlying(CredentialTypeEnum::kPin)][i].status == DlCredentialStatus::kAvailable)
+        if (currentCredential.status == DlCredentialStatus::kAvailable)
         {
             continue;
         }
 
-        if (mLockCredentials[to_underlying(CredentialTypeEnum::kPin)][i].credentialData.data_equal(pin.Value()))
+        if (currentCredential.credentialData.data_equal(pin.Value()))
         {
             ChipLogDetail(Zcl,
                           "Lock App: specified PIN code was found in the database, setting lock state to \"%s\" [endpointId=%d]",

diff --git a/examples/lock-app/silabs/efr32/include/LockManager.h b/examples/lock-app/silabs/efr32/include/LockManager.h
@@ -58,6 +58,8 @@ static constexpr uint8_t kMaxHolidaySchedules        = 10;
 static constexpr uint8_t kMaxCredentialSize          = 20;
 static constexpr uint8_t kNumCredentialTypes         = 6;
 
+static constexpr uint8_t kMaxCredentials = kMaxUsers * kMaxCredentialsPerUser;
+
 } // namespace ResourceRanges
 
 namespace LockInitParams {
@@ -175,6 +177,7 @@ class LockManager
 
     bool IsValidUserIndex(uint16_t userIndex);
     bool IsValidCredentialIndex(uint16_t credentialIndex, CredentialTypeEnum type);
+    bool IsValidCredentialType(CredentialTypeEnum type);
     bool IsValidWeekdayScheduleIndex(uint8_t scheduleIndex);
     bool IsValidYeardayScheduleIndex(uint8_t scheduleIndex);
     bool IsValidHolidayScheduleIndex(uint8_t scheduleIndex);
@@ -200,14 +203,14 @@ class LockManager
     static void ActuatorMovementTimerEventHandler(AppEvent * aEvent);
 
     EmberAfPluginDoorLockUserInfo mLockUsers[kMaxUsers];
-    EmberAfPluginDoorLockCredentialInfo mLockCredentials[kNumCredentialTypes][kMaxCredentialsPerUser];
+    EmberAfPluginDoorLockCredentialInfo mLockCredentials[kNumCredentialTypes][kMaxCredentials];
     WeekDaysScheduleInfo mWeekdaySchedule[kMaxUsers][kMaxWeekdaySchedulesPerUser];
     YearDayScheduleInfo mYeardaySchedule[kMaxUsers][kMaxYeardaySchedulesPerUser];
     HolidayScheduleInfo mHolidaySchedule[kMaxHolidaySchedules];
 
     char mUserNames[ArraySize(mLockUsers)][DOOR_LOCK_MAX_USER_NAME_SIZE];
-    uint8_t mCredentialData[kNumCredentialTypes][kMaxCredentialsPerUser][kMaxCredentialSize];
-    CredentialStruct mCredentials[kMaxUsers][kMaxCredentialsPerUser];
+    uint8_t mCredentialData[kNumCredentialTypes][kMaxCredentials][kMaxCredentialSize];
+    CredentialStruct mCredentials[kMaxUsers][kMaxCredentials];
 
     static LockManager sLock;
     EFR32DoorLock::LockInitParams::LockParam LockParams;

diff --git a/examples/lock-app/silabs/efr32/src/LockManager.cpp b/examples/lock-app/silabs/efr32/src/LockManager.cpp
@@ -113,6 +113,11 @@ bool LockManager::IsValidCredentialIndex(uint16_t credentialIndex, CredentialTyp
     return (credentialIndex < kMaxCredentialsPerUser);
 }
 
+bool LockManager::IsValidCredentialType(CredentialTypeEnum type)
+{
+    return (to_underlying(type) < kNumCredentialTypes);
+}
+
 bool LockManager::IsValidWeekdayScheduleIndex(uint8_t scheduleIndex)
 {
     return (scheduleIndex < kMaxWeekdaySchedulesPerUser);
@@ -134,9 +139,8 @@ bool LockManager::ReadConfigValues()
     SilabsConfig::ReadConfigValueBin(SilabsConfig::kConfigKey_LockUser, reinterpret_cast<uint8_t *>(&mLockUsers),
                                      sizeof(EmberAfPluginDoorLockUserInfo) * ArraySize(mLockUsers), outLen);
 
-    SilabsConfig::ReadConfigValueBin(
-        SilabsConfig::kConfigKey_Credential, reinterpret_cast<uint8_t *>(&mLockCredentials),
-        sizeof(EmberAfPluginDoorLockCredentialInfo) * LockParams.numberOfCredentialsPerUser * kNumCredentialTypes, outLen);
+    SilabsConfig::ReadConfigValueBin(SilabsConfig::kConfigKey_Credential, reinterpret_cast<uint8_t *>(&mLockCredentials),
+                                     sizeof(EmberAfPluginDoorLockCredentialInfo) * kMaxCredentials * kNumCredentialTypes, outLen);
 
     SilabsConfig::ReadConfigValueBin(SilabsConfig::kConfigKey_LockUserName, reinterpret_cast<uint8_t *>(mUserNames),
                                      sizeof(mUserNames), outLen);
@@ -379,9 +383,7 @@ bool LockManager::SetUser(chip::EndpointId endpointId, uint16_t userIndex, chip:
 
     for (size_t i = 0; i < totalCredentials; ++i)
     {
-        mCredentials[userIndex][i]                 = credentials[i];
-        mCredentials[userIndex][i].credentialType  = credentials[i].credentialType;
-        mCredentials[userIndex][i].credentialIndex = credentials[i].credentialIndex;
+        mCredentials[userIndex][i] = credentials[i];
     }
 
     userInStorage.credentials = chip::Span<const CredentialStruct>(mCredentials[userIndex], totalCredentials);
@@ -405,6 +407,8 @@ bool LockManager::GetCredential(chip::EndpointId endpointId, uint16_t credential
                                 EmberAfPluginDoorLockCredentialInfo & credential)
 {
 
+    VerifyOrReturnValue(IsValidCredentialType(credentialType), false);
+
     if (CredentialTypeEnum::kProgrammingPIN == credentialType)
     {
         VerifyOrReturnValue(IsValidCredentialIndex(credentialIndex, credentialType),
@@ -448,6 +452,8 @@ bool LockManager::SetCredential(chip::EndpointId endpointId, uint16_t credential
                                 const chip::ByteSpan & credentialData)
 {
 
+    VerifyOrReturnValue(IsValidCredentialType(credentialType), false);
+
     if (CredentialTypeEnum::kProgrammingPIN == credentialType)
     {
         VerifyOrReturnValue(IsValidCredentialIndex(credentialIndex, credentialType),
@@ -476,8 +482,7 @@ bool LockManager::SetCredential(chip::EndpointId endpointId, uint16_t credential
 
     // Save credential information in NVM flash
     SilabsConfig::WriteConfigValueBin(SilabsConfig::kConfigKey_Credential, reinterpret_cast<const uint8_t *>(&mLockCredentials),
-                                      sizeof(EmberAfPluginDoorLockCredentialInfo) * LockParams.numberOfCredentialsPerUser *
-                                          kNumCredentialTypes);
+                                      sizeof(EmberAfPluginDoorLockCredentialInfo) * kMaxCredentials * kNumCredentialTypes);
 
     SilabsConfig::WriteConfigValueBin(SilabsConfig::kConfigKey_CredentialData, reinterpret_cast<const uint8_t *>(&mCredentialData),
                                       sizeof(mCredentialData));
@@ -682,15 +687,15 @@ bool LockManager::setLockState(chip::EndpointId endpointId, DlLockState lockStat
     }
 
     // Check the PIN code
-    for (uint8_t i = 0; i < kMaxCredentialsPerUser; i++)
+    for (const auto & currentCredential : mLockCredentials[to_underlying(CredentialTypeEnum::kPin)])
     {
 
-        if (mLockCredentials[to_underlying(CredentialTypeEnum::kPin)][i].status == DlCredentialStatus::kAvailable)
+        if (currentCredential.status == DlCredentialStatus::kAvailable)
         {
             continue;
         }
 
-        if (mLockCredentials[to_underlying(CredentialTypeEnum::kPin)][i].credentialData.data_equal(pin.Value()))
+        if (currentCredential.credentialData.data_equal(pin.Value()))
         {
             ChipLogDetail(Zcl,
                           "Lock App: specified PIN code was found in the database, setting lock state to \"%s\" [endpointId=%d]",