From 4c6a6120c2b8b958ab391892d5eb538ddc45e866 Mon Sep 17 00:00:00 2001 From: Jakub Latusek Date: Fri, 12 Jul 2024 06:37:20 +0200 Subject: [PATCH] Fix codeql errors (#34248) * Fix error with not checking results of snprintf * Fix codeql errors --- .../decoder/logging/ToCertificateString.cpp | 23 +++++++++++++------ src/app/WriteClient.h | 2 +- src/app/tests/TestBufferedReadCallback.cpp | 4 ++-- src/lib/shell/commands/Config.cpp | 2 +- src/lib/shell/commands/Dns.cpp | 6 ++--- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/examples/common/tracing/decoder/logging/ToCertificateString.cpp b/examples/common/tracing/decoder/logging/ToCertificateString.cpp index 423544a9cc2a0c..1abd27ef788c59 100644 --- 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(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(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 index b67bf4ea0b8c84..b4d803981be3d8 100644 --- a/src/app/WriteClient.h +++ b/src/app/WriteClient.h @@ -178,7 +178,7 @@ class WriteClient : public Messaging::ExchangeDelegate ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, DataModel::List())); 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 index c16983c9c5605f..f3afeb7715cee3 100644 --- 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 index e1452f9b35ba67..cf5ac39e07983f 100644 --- 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 index 2c7cc02c7a54b2..18e0841dccce83 100644 --- 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");