Skip to content

Commit

Permalink
Replaced str... with strn... methods
Browse files Browse the repository at this point in the history
Static code analysis shown that there are several places that
str.. methods are used, what may lead to the bad memory access.
Replaced all of them with corresponding strn... methods.
  • Loading branch information
kkasperczyk-no committed Feb 21, 2025
1 parent 4fa81f6 commit e4d6dd4
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ CHIP_ERROR Server::SendUserDirectedCommissioningRequest(Transport::PeerAddress c
CHIP_ERROR err;

// only populate fields left blank by the client
if (strlen(id.GetInstanceName()) == 0)
if (strnlen(id.GetInstanceName(), Dnssd::Commission::kInstanceNameMaxLength + 1) == 0)
{
ChipLogDetail(AppServer, "Server::SendUserDirectedCommissioningRequest() Instance Name not known");
char nameBuffer[Dnssd::Commission::kInstanceNameMaxLength + 1];
Expand Down Expand Up @@ -738,7 +738,7 @@ CHIP_ERROR Server::SendUserDirectedCommissioningRequest(Transport::PeerAddress c
}
}

if (strlen(id.GetDeviceName()) == 0)
if (strnlen(id.GetDeviceName(), Dnssd::kKeyDeviceNameMaxLength + 1) == 0)
{
char deviceName[Dnssd::kKeyDeviceNameMaxLength + 1] = {};
if (!DeviceLayer::ConfigurationMgr().IsCommissionableDeviceNameEnabled() ||
Expand Down
17 changes: 9 additions & 8 deletions src/inet/InetInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ namespace Inet {
#if CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT
CHIP_ERROR InterfaceId::GetInterfaceName(char * nameBuf, size_t nameBufSize) const
{
if (mPlatformInterface && nameBufSize >= kMaxIfNameLength)
if (mPlatformInterface && nameBufSize >= InterfaceId::kMaxIfNameLength)
{
nameBuf[0] = 'o';
nameBuf[1] = 't';
Expand All @@ -84,7 +84,7 @@ CHIP_ERROR InterfaceId::GetInterfaceName(char * nameBuf, size_t nameBufSize) con
}
CHIP_ERROR InterfaceId::InterfaceNameToId(const char * intfName, InterfaceId & interface)
{
if (strlen(intfName) < 3)
if (strnlen(intfName, kMaxIfNameLength) < 3)
{
return INET_ERROR_UNKNOWN_INTERFACE;
}
Expand Down Expand Up @@ -210,7 +210,7 @@ CHIP_ERROR InterfaceId::GetInterfaceName(char * nameBuf, size_t nameBufSize) con

CHIP_ERROR InterfaceId::InterfaceNameToId(const char * intfName, InterfaceId & interface)
{
if (strlen(intfName) < 3)
if (strnlen(intfName, InterfaceId::kMaxIfNameLength) < 3)
{
return INET_ERROR_UNKNOWN_INTERFACE;
}
Expand Down Expand Up @@ -445,7 +445,7 @@ CHIP_ERROR InterfaceId::GetInterfaceName(char * nameBuf, size_t nameBufSize) con
{
return CHIP_ERROR_POSIX(errno);
}
size_t nameLength = strlen(intfName);
size_t nameLength = strnlen(intfName, IF_NAMESIZE);
if (nameLength >= nameBufSize)
{
return CHIP_ERROR_BUFFER_TOO_SMALL;
Expand Down Expand Up @@ -579,7 +579,8 @@ InterfaceId InterfaceIterator::GetInterfaceId()
CHIP_ERROR InterfaceIterator::GetInterfaceName(char * nameBuf, size_t nameBufSize)
{
VerifyOrReturnError(HasCurrent(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(strlen(mIntfArray[mCurIntf].if_name) < nameBufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
VerifyOrReturnError(strnlen(mIntfArray[mCurIntf].if_name, InterfaceId::kMaxIfNameLength) < nameBufSize,
CHIP_ERROR_BUFFER_TOO_SMALL);
Platform::CopyString(nameBuf, nameBufSize, mIntfArray[mCurIntf].if_name);
return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -728,7 +729,7 @@ InterfaceId InterfaceAddressIterator::GetInterfaceId()
CHIP_ERROR InterfaceAddressIterator::GetInterfaceName(char * nameBuf, size_t nameBufSize)
{
VerifyOrReturnError(HasCurrent(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(strlen(mCurAddr->ifa_name) < nameBufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
VerifyOrReturnError(strnlen(mCurAddr->ifa_name, InterfaceId::kMaxIfNameLength) < nameBufSize, CHIP_ERROR_BUFFER_TOO_SMALL);
Platform::CopyString(nameBuf, nameBufSize, mCurAddr->ifa_name);
return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -802,7 +803,7 @@ CHIP_ERROR InterfaceId::GetInterfaceName(char * nameBuf, size_t nameBufSize) con
return CHIP_ERROR_INCORRECT_STATE;
}
const char * name = net_if_get_device(currentInterface)->name;
size_t nameLength = strlen(name);
size_t nameLength = strnlen(name, InterfaceId::kMaxIfNameLength);
if (nameLength >= nameBufSize)
{
return CHIP_ERROR_BUFFER_TOO_SMALL;
Expand All @@ -825,7 +826,7 @@ CHIP_ERROR InterfaceId::InterfaceNameToId(const char * intfName, InterfaceId & i

while ((currentInterface = net_if_get_by_index(++currentId)) != nullptr)
{
if (strcmp(net_if_get_device(currentInterface)->name, intfName) == 0)
if (strncmp(net_if_get_device(currentInterface)->name, intfName, InterfaceId::kMaxIfNameLength) == 0)
{
interface = InterfaceId(currentId);
return CHIP_NO_ERROR;
Expand Down
3 changes: 2 additions & 1 deletion src/inet/UDPEndPointImplSockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ CHIP_ERROR UDPEndPointImplSockets::BindInterfaceImpl(IPAddressType addressType,
{
status = CHIP_ERROR_POSIX(errno);
}
else if (setsockopt(mSocket, SOL_SOCKET, SO_BINDTODEVICE, interfaceName, socklen_t(strlen(interfaceName))) == -1)
else if (setsockopt(mSocket, SOL_SOCKET, SO_BINDTODEVICE, interfaceName, socklen_t(strnlen(interfaceName, IF_NAMESIZE))) ==
-1)
{
status = CHIP_ERROR_POSIX(errno);
}
Expand Down
5 changes: 3 additions & 2 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,16 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser
auto & ipAddress = services[i].mAddress;

// mType(service name) exactly matches with operational service name
bool isOperationalBrowse = strcmp(services[i].mType, kOperationalServiceName) == 0;
bool isOperationalBrowse = strncmp(services[i].mType, kOperationalServiceName, kDnssdTypeMaxSize + 1) == 0;

// For operational browse result we currently don't need IP address hence skip resolution and handle differently.
if (isOperationalBrowse)
{
HandleNodeOperationalBrowse(context, &services[i], error);
}
// check whether SRV, TXT and AAAA records were received in DNS responses
else if (strlen(services[i].mHostName) == 0 || services[i].mTextEntrySize == 0 || !ipAddress.has_value())
else if (strnlen(services[i].mHostName, chip::Dnssd::kHostNameMaxLength + 1) == 0 || services[i].mTextEntrySize == 0 ||
!ipAddress.has_value())
{
ChipDnssdResolve(&services[i], services[i].mInterface, HandleNodeResolve, context);
}
Expand Down
14 changes: 9 additions & 5 deletions src/lib/dnssd/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ struct DiscoveryFilter
}
if (type == DiscoveryFilterType::kInstanceName)
{
return (instanceName != nullptr) && (other.instanceName != nullptr) && (strcmp(instanceName, other.instanceName) == 0);
return (instanceName != nullptr) && (other.instanceName != nullptr) &&
(strncmp(instanceName, other.instanceName, Common::kInstanceNameMaxLength + 1) == 0);
}

return code == other.code;
Expand Down Expand Up @@ -121,7 +122,7 @@ struct CommonResolutionData
(mrpRetryIntervalActive.has_value() && (*mrpRetryIntervalActive > defaultMRPConfig->mActiveRetransTimeout));
}

bool IsHost(const char * host) const { return strcmp(host, hostName) == 0; }
bool IsHost(const char * host) const { return strncmp(host, hostName, kHostNameMaxLength + 1) == 0; }

void Reset()
{
Expand Down Expand Up @@ -244,7 +245,10 @@ struct CommissionNodeData : public CommonResolutionData
new (this) CommissionNodeData();
}

bool IsInstanceName(const char * instance) const { return strcmp(instance, instanceName) == 0; }
bool IsInstanceName(const char * instance) const
{
return strncmp(instance, instanceName, Common::kInstanceNameMaxLength + 1) == 0;
}

void LogDetail() const
{
Expand All @@ -257,7 +261,7 @@ struct CommissionNodeData : public CommonResolutionData
Encoding::BytesToUppercaseHexString(rotatingId, rotatingIdLen, rotatingIdString, sizeof(rotatingIdString));
ChipLogDetail(Discovery, "\tRotating ID: %s", rotatingIdString);
}
if (strlen(deviceName) != 0)
if (strnlen(deviceName, kMaxDeviceNameLen + 1) != 0)
{
ChipLogDetail(Discovery, "\tDevice Name: %s", deviceName);
}
Expand All @@ -277,7 +281,7 @@ struct CommissionNodeData : public CommonResolutionData
{
ChipLogDetail(Discovery, "\tLong Discriminator: %u", longDiscriminator);
}
if (strlen(pairingInstruction) != 0)
if (strnlen(pairingInstruction, kMaxPairingInstructionLen + 1) != 0)
{
ChipLogDetail(Discovery, "\tPairing Instruction: %s", pairingInstruction);
}
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Zephyr/BLEManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ inline CHIP_ERROR BLEManagerImpl::PrepareAdvertisingRequest()
static_assert(sizeof(serviceData) == 10, "Unexpected size of BLE advertising data!");

const char * name = bt_get_name();
const uint8_t nameSize = static_cast<uint8_t>(strlen(name));
const uint8_t nameSize = static_cast<uint8_t>(strnlen(name, CONFIG_BT_DEVICE_NAME_MAX));

Encoding::LittleEndian::Put16(serviceData.uuid, UUID16_CHIPoBLEService.val);
ReturnErrorOnFailure(ConfigurationMgr().GetBLEDeviceIdentificationInfo(serviceData.deviceIdInfo));
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Zephyr/KeyValueStoreManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ constexpr size_t kEmptyValueSize = sizeof(kEmptyValue);
CHIP_ERROR MakeFullKey(char (&fullKey)[SETTINGS_MAX_NAME_LEN + 1], const char * key)
{
VerifyOrReturnError(key != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
strcpy(fullKey, CHIP_DEVICE_CONFIG_SETTINGS_KEY "/");
strncpy(fullKey, CHIP_DEVICE_CONFIG_SETTINGS_KEY "/", SETTINGS_MAX_NAME_LEN + 1);

char * dest = fullKey + strlen(CHIP_DEVICE_CONFIG_SETTINGS_KEY "/");
char * destEnd = fullKey + SETTINGS_MAX_NAME_LEN;
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Zephyr/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char
char formattedMsg[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
snprintfcb(formattedMsg, sizeof(formattedMsg), "[%s]", module);

const size_t prefixLen = strlen(formattedMsg);
const size_t prefixLen = strnlen(formattedMsg, CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE);
vsnprintfcb(formattedMsg + prefixLen, sizeof(formattedMsg) - prefixLen, msg, v);

const char * allocatedMsg = formattedMsg;
Expand Down
4 changes: 3 additions & 1 deletion src/platform/Zephyr/ZephyrConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <platform/internal/testing/ConfigUnitTest.h>

#include <zephyr/settings/settings.h>
#include <zephyr/storage/flash_map.h>

namespace chip {
namespace DeviceLayer {
Expand Down Expand Up @@ -229,6 +230,7 @@ CHIP_ERROR ZephyrConfig::WriteConfigValue(Key key, uint64_t val)

CHIP_ERROR ZephyrConfig::WriteConfigValueStr(Key key, const char * str)
{
/* The API contract requires using null-terminated string, so using strlen here should be safe. */
return WriteConfigValueStr(key, str, str ? strlen(str) : 0);
}

Expand Down Expand Up @@ -291,7 +293,7 @@ void ZephyrConfig::RunConfigUnitTest()
bool ZephyrConfig::BuildCounterConfigKey(::chip::Platform::PersistedStorage::Key counterId, char key[SETTINGS_MAX_NAME_LEN])
{
constexpr size_t KEY_PREFIX_LEN = sizeof(NAMESPACE_COUNTERS) - 1;
const size_t keySuffixLen = strlen(counterId) + 1; // including null-character
const size_t keySuffixLen = strnlen(counterId, SETTINGS_MAX_NAME_LEN) + 1; // including null-character

if (KEY_PREFIX_LEN + keySuffixLen > SETTINGS_MAX_NAME_LEN)
return false;
Expand Down

0 comments on commit e4d6dd4

Please sign in to comment.