Skip to content

Commit 5a9e314

Browse files
authored
Fixing UBSan issues that showed up in all-clusters app (#35580)
* Fix for unsigned integer overflow: Error Message: BufferWriter.cpp:76:16: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long') when size becomes 0 and we are in while (size-- > 0), then size will become less than 0, which should be avoided since it is an unsigned (although technically it wraps around, UBSAN complains about it). Fix: stop size from decrement past 0, by moving size-- inside the loop * Fix null pointer passed to non-null argument in CHIPMemString.h Error Message: CHIPMemString.h:88:22: runtime error: null pointer passed as argument 2, which is declared to never be null when PI= is in mDNS TXT record (its value is empty) , and Dnssd::Internal::GetPairingInstruction calls CopyString, source is an empty bytespan and source.data() will return a null pointer, that will be passed to memcpy Fix: avoid memcpy in that case.
1 parent 32200d1 commit 5a9e314

File tree

2 files changed

+16
-6
lines changed

2 files changed

+16
-6
lines changed

src/lib/support/BufferWriter.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,19 @@ LittleEndian::BufferWriter & LittleEndian::BufferWriter::EndianPutSigned(int64_t
7373

7474
BigEndian::BufferWriter & BigEndian::BufferWriter::EndianPut(uint64_t x, size_t size)
7575
{
76-
while (size-- > 0)
76+
while (size > 0)
7777
{
78+
size--;
7879
Put(static_cast<uint8_t>((x >> (size * 8)) & 0xff));
7980
}
8081
return *this;
8182
}
8283

8384
BigEndian::BufferWriter & BigEndian::BufferWriter::EndianPutSigned(int64_t x, size_t size)
8485
{
85-
while (size-- > 0)
86+
while (size > 0)
8687
{
88+
size--;
8789
Put(static_cast<uint8_t>((x >> (size * 8)) & 0xff));
8890
}
8991
return *this;

src/lib/support/CHIPMemString.h

+12-4
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,20 @@ inline void CopyString(char (&dest)[N], const char * source)
8282
*/
8383
inline void CopyString(char * dest, size_t destLength, ByteSpan source)
8484
{
85-
if (dest && destLength)
85+
if ((dest == nullptr) || (destLength == 0))
8686
{
87-
size_t maxChars = std::min(destLength - 1, source.size());
88-
memcpy(dest, source.data(), maxChars);
89-
dest[maxChars] = '\0';
87+
return; // no space to copy anything, not even a null terminator
9088
}
89+
90+
if (source.empty())
91+
{
92+
*dest = '\0'; // just a null terminator, we are copying empty data
93+
return;
94+
}
95+
96+
size_t maxChars = std::min(destLength - 1, source.size());
97+
memcpy(dest, source.data(), maxChars);
98+
dest[maxChars] = '\0';
9199
}
92100

93101
/**

0 commit comments

Comments
 (0)