Fix codeql errors (#34248)

* Fix error with not checking results of snprintf * Fix codeql errors
project-chip · Jul 12, 2024 · 4c6a612 · 4c6a612
1 parent d856745
commit 4c6a612
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 14 deletions.
diff --git a/examples/common/tracing/decoder/logging/ToCertificateString.cpp b/examples/common/tracing/decoder/logging/ToCertificateString.cpp
@@ -35,7 +35,7 @@ const char * ToCertificate(const chip::ByteSpan & source, chip::MutableCharSpan
 {
     // Reset the buffer
     memset(destination.data(), '\0', destination.size());
-
+    int snprintf_len = 0;
     if (source.size() == 0)
     {
         return destination.data();
@@ -70,7 +70,8 @@ const char * ToCertificate(const chip::ByteSpan & source, chip::MutableCharSpan
             ChipLogError(DataManagement, "Certificate size is greater than 400 bytes");
         }
 
-        snprintf(destination.data(), destination.size(), "%s", str.Get());
+        snprintf_len = snprintf(destination.data(), destination.size(), "%s", str.Get());
+        VerifyOrExit(snprintf_len >= 0, ChipLogError(DataManagement, "Failed to write certificate"););
     }
     else
     {
@@ -83,15 +84,23 @@ const char * ToCertificate(const chip::ByteSpan & source, chip::MutableCharSpan
         size_t inIndex  = 0;
         size_t outIndex = strlen(header) + 1;
 
-        snprintf(destination.data(), destination.size(), "%s\n", header);
+        snprintf_len = snprintf(destination.data(), destination.size(), "%s\n", header);
+        VerifyOrExit(snprintf_len >= 0, ChipLogError(DataManagement, "Failed to write header"););
         for (; inIndex < base64DataLen; inIndex += 64)
         {
-            auto charsPrinted = snprintf(&destination.data()[outIndex], destination.size() - outIndex, "%.64s\n", &str[inIndex]);
-            outIndex += static_cast<size_t>(charsPrinted);
+            snprintf_len = snprintf(&destination.data()[outIndex], destination.size() - outIndex, "%.64s\n", &str[inIndex]);
+            VerifyOrExit(snprintf_len >= 0, ChipLogError(DataManagement, "Failed to write certificate"););
+
+            outIndex += static_cast<size_t>(snprintf_len);
         }
-        snprintf(&destination.data()[outIndex], destination.size() - outIndex, "%s", footer);
+        snprintf_len = snprintf(&destination.data()[outIndex], destination.size() - outIndex, "%s", footer);
+        VerifyOrExit(snprintf_len >= 0, ChipLogError(DataManagement, "Failed to write footer"););
+    }
+exit:
+    if (snprintf_len < 0)
+    {
+        memset(destination.data(), '\0', destination.size());
     }
-
     return destination.data();
 }
 

diff --git a/src/app/WriteClient.h b/src/app/WriteClient.h
@@ -178,7 +178,7 @@ class WriteClient : public Messaging::ExchangeDelegate
         ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, DataModel::List<uint8_t>()));
 
         path.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem;
-        for (ListIndex i = 0; i < value.size(); i++)
+        for (size_t i = 0; i < value.size(); i++)
         {
             ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, value.data()[i]));
         }

diff --git a/src/app/tests/TestBufferedReadCallback.cpp b/src/app/tests/TestBufferedReadCallback.cpp
@@ -235,11 +235,11 @@ void DataSeriesValidator::OnAttributeData(const ConcreteDataAttributePath & aPat
 
         auto iter = value.begin();
 
-        uint8_t index = 0;
+        uint32_t index = 0;
         while (iter.Next() && index < expectedListLength)
         {
             auto & iterValue = iter.GetValue();
-            EXPECT_EQ(iterValue, (index));
+            EXPECT_EQ(iterValue, (index % 256));
             index++;
         }
 

diff --git a/src/lib/shell/commands/Config.cpp b/src/lib/shell/commands/Config.cpp
@@ -142,7 +142,7 @@ static CHIP_ERROR ConfigSetSetupDiscriminator(char * argv)
     }
     else
     {
-        streamer_printf(sout, "Setup discriminator setting failed with code: %d\r\n", error);
+        streamer_printf(sout, "Setup discriminator setting failed with code: %d\r\n", error.AsInteger());
     }
 
     return error;

diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp
@@ -122,18 +122,18 @@ class DnsShellResolverDelegate : public Dnssd::DiscoverNodeDelegate, public Addr
         auto retryInterval = nodeData.GetMrpRetryIntervalIdle();
 
         if (retryInterval.has_value())
-            streamer_printf(streamer_get(), "   MRP retry interval (idle): %" PRIu32 "ms\r\n", *retryInterval);
+            streamer_printf(streamer_get(), "   MRP retry interval (idle): %" PRIu32 "ms\r\n", retryInterval->count());
 
         retryInterval = nodeData.GetMrpRetryIntervalActive();
 
         if (retryInterval.has_value())
-            streamer_printf(streamer_get(), "   MRP retry interval (active): %" PRIu32 "ms\r\n", *retryInterval);
+            streamer_printf(streamer_get(), "   MRP retry interval (active): %" PRIu32 "ms\r\n", retryInterval->count());
 
         auto activeThreshold = nodeData.GetMrpRetryActiveThreshold();
 
         if (activeThreshold.has_value())
         {
-            streamer_printf(streamer_get(), "   MRP retry active threshold time: %" PRIu32 "ms\r\n", *activeThreshold);
+            streamer_printf(streamer_get(), "   MRP retry active threshold time: %" PRIu32 "ms\r\n", activeThreshold->count());
         }
 
         streamer_printf(streamer_get(), "   Supports TCP Client: %s\r\n", nodeData.supportsTcpClient ? "yes" : "no");