Skip to content

Commit 48303c5

Browse files
woody-appleclaudeandy31415
authored
Fix integer overflow in TLVReader::GetString bounds check (project-chip#71934)
* Fix integer overflow in TLVReader::GetString bounds check The bounds check `(mElemLenOrVal + 1) > bufSize` can wrap to 0 when mElemLenOrVal is large (e.g., UINT32_MAX on 32-bit targets), bypassing the buffer size validation and causing an out-of-bounds write of the null terminator. Replace with `mElemLenOrVal >= bufSize` which is overflow-safe and semantically equivalent. Also pass the actual element length to GetBytes instead of bufSize-1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add larger-buffer test case for GetString per review feedback Verifies that GetString with a buffer larger than the string content succeeds correctly, confirming that GetBytes is called with the actual element length rather than bufSize-1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Retrigger CI (REPL timeout) * Retrigger CI (nRF/REPL flake) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Andrei Litvin <andy314@gmail.com>
1 parent 20fcb76 commit 48303c5

2 files changed

Lines changed: 65 additions & 2 deletions

File tree

src/lib/core/TLVReader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,12 +441,12 @@ CHIP_ERROR TLVReader::GetString(char * buf, size_t bufSize)
441441
if (!TLVTypeIsString(ElementType()))
442442
return CHIP_ERROR_WRONG_TLV_TYPE;
443443

444-
if ((mElemLenOrVal + 1) > bufSize)
444+
if (mElemLenOrVal >= bufSize)
445445
return CHIP_ERROR_BUFFER_TOO_SMALL;
446446

447447
buf[mElemLenOrVal] = 0;
448448

449-
return GetBytes(reinterpret_cast<uint8_t *>(buf), bufSize - 1);
449+
return GetBytes(reinterpret_cast<uint8_t *>(buf), static_cast<uint32_t>(mElemLenOrVal));
450450
}
451451

452452
CHIP_ERROR TLVReader::DupBytes(uint8_t *& buf, uint32_t & dataLen)

src/lib/core/tests/TestTLV.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4946,3 +4946,66 @@ TEST_F(TestTLV, TestUninitializedWriter)
49464946
EXPECT_EQ(writer.CopyContainer(ContextTag(1), buf, static_cast<uint16_t>(sizeof(buf))), CHIP_ERROR_INCORRECT_STATE);
49474947
}
49484948
}
4949+
4950+
TEST_F(TestTLV, CheckGetStringBoundsCheck)
4951+
{
4952+
// Write a short UTF8 string "Hi" into TLV
4953+
uint8_t backingStore[32];
4954+
TLVWriter writer;
4955+
writer.Init(backingStore, sizeof(backingStore));
4956+
4957+
CHIP_ERROR err = writer.PutString(AnonymousTag(), "Hi");
4958+
EXPECT_EQ(err, CHIP_NO_ERROR);
4959+
err = writer.Finalize();
4960+
EXPECT_EQ(err, CHIP_NO_ERROR);
4961+
4962+
// Reading with exact-fit buffer (3 bytes: 'H', 'i', '\0') should succeed
4963+
{
4964+
TLVReader reader;
4965+
reader.Init(backingStore, writer.GetLengthWritten());
4966+
err = reader.Next();
4967+
EXPECT_EQ(err, CHIP_NO_ERROR);
4968+
4969+
char buf[3];
4970+
err = reader.GetString(buf, sizeof(buf));
4971+
EXPECT_EQ(err, CHIP_NO_ERROR);
4972+
EXPECT_STREQ(buf, "Hi");
4973+
}
4974+
4975+
// Reading with buffer larger than needed should succeed
4976+
{
4977+
TLVReader reader;
4978+
reader.Init(backingStore, writer.GetLengthWritten());
4979+
err = reader.Next();
4980+
EXPECT_EQ(err, CHIP_NO_ERROR);
4981+
4982+
char buf[10];
4983+
err = reader.GetString(buf, sizeof(buf));
4984+
EXPECT_EQ(err, CHIP_NO_ERROR);
4985+
EXPECT_STREQ(buf, "Hi");
4986+
}
4987+
4988+
// Reading with buffer too small by 1 byte should fail
4989+
{
4990+
TLVReader reader;
4991+
reader.Init(backingStore, writer.GetLengthWritten());
4992+
err = reader.Next();
4993+
EXPECT_EQ(err, CHIP_NO_ERROR);
4994+
4995+
char buf[2];
4996+
err = reader.GetString(buf, sizeof(buf));
4997+
EXPECT_EQ(err, CHIP_ERROR_BUFFER_TOO_SMALL);
4998+
}
4999+
5000+
// Reading with bufSize=0 should fail
5001+
{
5002+
TLVReader reader;
5003+
reader.Init(backingStore, writer.GetLengthWritten());
5004+
err = reader.Next();
5005+
EXPECT_EQ(err, CHIP_NO_ERROR);
5006+
5007+
char buf[1];
5008+
err = reader.GetString(buf, 0);
5009+
EXPECT_EQ(err, CHIP_ERROR_BUFFER_TOO_SMALL);
5010+
}
5011+
}

0 commit comments

Comments
 (0)