Fix out-of-bound read when passing CharSpan to printf (#15796)

The CharSpan object is not a C-style string - it is not terminated with null byte. When using such object with printf-like functions we have to make sure that the length of the string will be passed as well (using "%.*s" syntax). Otherwise, we might face out-of-bound reads which is a security threat.
project-chip · Nov 30, 2023 · 1060344 · 1060344
1 parent ff6e83c
commit 1060344
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 22 deletions.
diff --git a/examples/door-lock-app/linux/src/LockEndpoint.cpp b/examples/door-lock-app/linux/src/LockEndpoint.cpp
@@ -60,7 +60,7 @@ bool LockEndpoint::GetUser(uint16_t userIndex, EmberAfPluginDoorLockUserInfo & u
 
     ChipLogDetail(Zcl,
                   "Found occupied user "
-                  "[endpoint=%d,adjustedIndex=%hu,name=\"%*.s\",credentialsCount=%zu,uniqueId=%x,type=%u,credentialRule=%u,"
+                  "[endpoint=%d,adjustedIndex=%hu,name=\"%.*s\",credentialsCount=%zu,uniqueId=%x,type=%u,credentialRule=%u,"
                   "createdBy=%d,lastModifiedBy=%d]",
                   mEndpointId, adjustedUserIndex, static_cast<int>(user.userName.size()), user.userName.data(),
                   user.credentials.size(), user.userUniqueId, to_underlying(user.userType), to_underlying(user.credentialRule),
@@ -75,7 +75,7 @@ bool LockEndpoint::SetUser(uint16_t userIndex, chip::FabricIndex creator, chip::
 {
     ChipLogProgress(Zcl,
                     "Door Lock App: LockEndpoint::SetUser "
-                    "[endpoint=%d,userIndex=%" PRIu16 ",creator=%d,modifier=%d,userName=\"%*.s\",uniqueId=%" PRIx32
+                    "[endpoint=%d,userIndex=%" PRIu16 ",creator=%d,modifier=%d,userName=\"%.*s\",uniqueId=%" PRIx32
                     ",userStatus=%u,userType=%u,"
                     "credentialRule=%u,credentials=%p,totalCredentials=%zu]",
                     mEndpointId, userIndex, creator, modifier, static_cast<int>(userName.size()), userName.data(), uniqueId,

diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp
@@ -308,11 +308,11 @@ void DoorLockServer::GetUserCommandHandler(chip::app::CommandHandler * commandOb
         if (DlUserStatus::kAvailable != user.userStatus)
         {
             emberAfDoorLockClusterPrintln("Found user in storage: "
-                                          "[userIndex=%d,userName=\"%s\",userStatus=%u,userType=%u"
+                                          "[userIndex=%d,userName=\"%.*s\",userStatus=%u,userType=%u"
                                           ",credentialRule=%u,createdBy=%u,modifiedBy=%u]",
-                                          userIndex, user.userName.data(), to_underlying(user.userStatus),
-                                          to_underlying(user.userType), to_underlying(user.credentialRule), user.createdBy,
-                                          user.lastModifiedBy);
+                                          userIndex, static_cast<int>(user.userName.size()), user.userName.data(),
+                                          to_underlying(user.userStatus), to_underlying(user.userType),
+                                          to_underlying(user.credentialRule), user.createdBy, user.lastModifiedBy);
 
             SuccessOrExit(err = writer->PutString(TLV::ContextTag(to_underlying(ResponseFields::kUserName)), user.userName));
             if (0xFFFFFFFFU != user.userUniqueId)
@@ -1541,19 +1541,19 @@ EmberAfStatus DoorLockServer::createUser(chip::EndpointId endpointId, chip::Fabr
     {
         emberAfDoorLockClusterPrintln(
             "[createUser] Unable to create user: app error "
-            "[endpointId=%d,creatorFabricId=%d,userIndex=%d,userName=\"%s\",userUniqueId=0x%" PRIx32 ",userStatus="
-            "%u,userType=%u,credentialRule=%u,totalCredentials=%zu]",
-            endpointId, creatorFabricIdx, userIndex, newUserName.data(), newUserUniqueId, to_underlying(newUserStatus),
-            to_underlying(newUserType), to_underlying(newCredentialRule), newTotalCredentials);
+            "[endpointId=%d,creatorFabricId=%d,userIndex=%d,userName=\"%.*s\",userUniqueId=0x%" PRIx32 ",userStatus=%u,"
+            "userType=%u,credentialRule=%u,totalCredentials=%zu]",
+            endpointId, creatorFabricIdx, userIndex, static_cast<int>(newUserName.size()), newUserName.data(), newUserUniqueId,
+            to_underlying(newUserStatus), to_underlying(newUserType), to_underlying(newCredentialRule), newTotalCredentials);
         return EMBER_ZCL_STATUS_FAILURE;
     }
 
     emberAfDoorLockClusterPrintln(
         "[createUser] User created "
-        "[endpointId=%d,creatorFabricId=%d,userIndex=%d,userName=\"%s\",userUniqueId=0x%" PRIx32 ",userStatus=%"
-        "u,userType=%u,credentialRule=%u,totalCredentials=%zu]",
-        endpointId, creatorFabricIdx, userIndex, newUserName.data(), newUserUniqueId, to_underlying(newUserStatus),
-        to_underlying(newUserType), to_underlying(newCredentialRule), newTotalCredentials);
+        "[endpointId=%d,creatorFabricId=%d,userIndex=%d,userName=\"%.*s\",userUniqueId=0x%" PRIx32 ",userStatus=%u,"
+        "userType=%u,credentialRule=%u,totalCredentials=%zu]",
+        endpointId, creatorFabricIdx, userIndex, static_cast<int>(newUserName.size()), newUserName.data(), newUserUniqueId,
+        to_underlying(newUserStatus), to_underlying(newUserType), to_underlying(newCredentialRule), newTotalCredentials);
 
     sendRemoteLockUserChange(endpointId, DlLockDataType::kUserIndex, DlDataOperationType::kAdd, sourceNodeId, creatorFabricIdx,
                              userIndex, userIndex);
@@ -1612,19 +1612,19 @@ EmberAfStatus DoorLockServer::modifyUser(chip::EndpointId endpointId, chip::Fabr
     {
         ChipLogError(Zcl,
                      "[modifyUser] Unable to modify the user: app error "
-                     "[endpointId=%d,modifierFabric=%d,userIndex=%d,userName=\"%s\",userUniqueId=0x%" PRIx32 ",userStatus=%u"
+                     "[endpointId=%d,modifierFabric=%d,userIndex=%d,userName=\"%.*s\",userUniqueId=0x%" PRIx32 ",userStatus=%u"
                      ",userType=%u,credentialRule=%u]",
-                     endpointId, modifierFabricIndex, userIndex, newUserName.data(), newUserUniqueId, to_underlying(newUserStatus),
-                     to_underlying(newUserType), to_underlying(newCredentialRule));
+                     endpointId, modifierFabricIndex, userIndex, static_cast<int>(newUserName.size()), newUserName.data(),
+                     newUserUniqueId, to_underlying(newUserStatus), to_underlying(newUserType), to_underlying(newCredentialRule));
         return EMBER_ZCL_STATUS_FAILURE;
     }
 
     emberAfDoorLockClusterPrintln("[modifyUser] User modified "
-                                  "[endpointId=%d,modifierFabric=%d,userIndex=%d,userName=\"%s\",userUniqueId=0x%" PRIx32
-                                  ",userStatus=%u,"
-                                  "userType=%u,credentialRule=%u]",
-                                  endpointId, modifierFabricIndex, userIndex, newUserName.data(), newUserUniqueId,
-                                  to_underlying(newUserStatus), to_underlying(newUserType), to_underlying(newCredentialRule));
+                                  "[endpointId=%d,modifierFabric=%d,userIndex=%d,userName=\"%.*s\",userUniqueId=0x%" PRIx32
+                                  ",userStatus=%u,userType=%u,credentialRule=%u]",
+                                  endpointId, modifierFabricIndex, userIndex, static_cast<int>(newUserName.size()),
+                                  newUserName.data(), newUserUniqueId, to_underlying(newUserStatus), to_underlying(newUserType),
+                                  to_underlying(newCredentialRule));
 
     sendRemoteLockUserChange(endpointId, DlLockDataType::kUserIndex, DlDataOperationType::kModify, sourceNodeId,
                              modifierFabricIndex, userIndex, userIndex);