Skip to content

Commit

Permalink
[coap] handle zero coap token length
Browse files Browse the repository at this point in the history
There could be potential out of bound error if the token length in CoAP
header is zero (`&aBuf[offset]` could be referring to the next element
after the end of the buffer).

Note that the added test can pass without the fix in coap.cpp but
it's probably because the toolchain has some optimization to avoid such
issues...
  • Loading branch information
wgtdkp committed Jul 4, 2024
1 parent 86e37dd commit 0a996ba
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/library/coap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,13 @@ Error Message::Deserialize(Header &aHeader, const ByteArray &aBuf, size_t &aOffs
header.mMessageId = aBuf[offset++];
header.mMessageId = (header.mMessageId << 8) | aBuf[offset++];

VerifyOrExit(offset + header.mTokenLength <= aBuf.size(),
error = ERROR_BAD_FORMAT("premature end of CoAP message header"));
memcpy(header.mToken, &aBuf[offset], std::min(header.mTokenLength, kMaxTokenLength));
offset += header.mTokenLength;
if (header.mTokenLength != 0)
{
VerifyOrExit(offset + header.mTokenLength <= aBuf.size(),
error = ERROR_BAD_FORMAT("premature end of CoAP message header"));
memcpy(header.mToken, &aBuf[offset], std::min(header.mTokenLength, kMaxTokenLength));
offset += header.mTokenLength;
}

aHeader = header;
aOffset = offset;
Expand Down
12 changes: 12 additions & 0 deletions src/library/coap_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ TEST(CoapTest, CoapMessageHeader_TokenIsPresent)
EXPECT_EQ(message->GetToken(), ByteArray{0xfa});
}

TEST(CoapTest, CoapMessageHeader_TokenLengthIsZero)
{
ByteArray buffer{'`', 0, 0, 1};
Error error;
auto message = Message::Deserialize(error, buffer);

EXPECT_NE(message, nullptr);
EXPECT_EQ(error, ErrorCode::kNone);

EXPECT_EQ(message->GetToken(), ByteArray{});
}

TEST(CoapTest, CoapMessageHeader_TokenLengthIsTooLong)
{
ByteArray buffer{0x49, 0x00, 0x00, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99};
Expand Down

0 comments on commit 0a996ba

Please sign in to comment.