[coap] reject option number overflow in Option::Iterator::Advance() (#13579)

Guards the cumulative option-number accumulation in
Option::Iterator::Advance() with CanAddSafely<uint16_t> so a delta
that would overflow past 0xFFFF is rejected with kErrorParse instead
of wrapping. Regression test is also added.
This commit is contained in:
Anurag Mewar
2026-09-18 13:22:39 -07:00
committed by GitHub
parent bb3b44994a
commit 22cc9c3632
2 changed files with 85 additions and 0 deletions
+1
View File
@@ -822,6 +822,7 @@ Error Option::Iterator::Advance(void)
optionDelta = (headerByte & Message::kOptionDeltaMask) >> Message::kOptionDeltaOffset;
SuccessOrExit(error = ReadExtendedOptionField(optionDelta));
VerifyOrExit(CanAddSafely<uint16_t>(mOption.mNumber, optionDelta), error = kErrorParse);
optionLength = (headerByte & Message::kOptionLengthMask) >> Message::kOptionLengthOffset;
SuccessOrExit(error = ReadExtendedOptionField(optionLength));
+84
View File
@@ -67,6 +67,88 @@ void TestCoapOverflow(void)
testFreeInstance(instance);
}
void TestCoapOptionNumberOverflow(void)
{
Instance *instance;
Coap::Message *message;
// Two options with deltas 65535 then 12. Each delta is individually
// valid, but their cumulative sum (65547) exceeds the 16-bit option
// number and would wrap to 11 (Uri-Path), producing a non-monotonic
// option sequence that RFC 7252 forbids.
//
// Byte 0xe0 : option 1, delta 14 (2-byte extension), length 0.
// Bytes 0xfe 0xf2 : extended delta 0xfef2 + 269 = 65535.
// Byte 0xc0 : option 2, delta 12, length 0.
static constexpr uint8_t kOptions[] = {0xe0, 0xfe, 0xf2, 0xc0};
printf("TestCoapOptionNumberOverflow()\n");
instance = static_cast<Instance *>(testInitInstance());
VerifyOrQuit(instance != nullptr);
message = AsCoapMessagePtr(instance->Get<MessagePool>().Allocate(Message::kTypeOther));
VerifyOrQuit(message != nullptr);
SuccessOrQuit(message->Init(Coap::kTypeNonConfirmable, Coap::kCodePut));
SuccessOrQuit(message->AppendBytes(kOptions, sizeof(kOptions)));
Coap::Option::Iterator iterator;
SuccessOrQuit(iterator.Init(*message));
VerifyOrQuit(!iterator.IsDone());
VerifyOrQuit(iterator.GetOption()->GetNumber() == 65535);
// The second option would wrap the running option number and must be rejected.
VerifyOrQuit(iterator.Advance() == kErrorParse, "Fix failed: cumulative option number overflow was not rejected!");
VerifyOrQuit(iterator.IsDone());
VerifyOrQuit(iterator.GetOption() == nullptr);
message->Free();
testFreeInstance(instance);
}
void TestCoapOptionNumberMaxRepeated(void)
{
Instance *instance;
Coap::Message *message;
// Option 1: delta 65535 (0xe0, 0xfe, 0xf2), length 0.
// Option 2: delta 0 (0x00), length 0.
// Verifies that repeated options with delta 0 at max option number 65535 are accepted.
static constexpr uint8_t kOptions[] = {0xe0, 0xfe, 0xf2, 0x00};
printf("TestCoapOptionNumberMaxRepeated()\n");
instance = static_cast<Instance *>(testInitInstance());
VerifyOrQuit(instance != nullptr);
message = AsCoapMessagePtr(instance->Get<MessagePool>().Allocate(Message::kTypeOther));
VerifyOrQuit(message != nullptr);
SuccessOrQuit(message->Init(Coap::kTypeNonConfirmable, Coap::kCodePut));
SuccessOrQuit(message->AppendBytes(kOptions, sizeof(kOptions)));
Coap::Option::Iterator iterator;
SuccessOrQuit(iterator.Init(*message));
VerifyOrQuit(!iterator.IsDone());
VerifyOrQuit(iterator.GetOption()->GetNumber() == 65535);
SuccessOrQuit(iterator.Advance());
VerifyOrQuit(!iterator.IsDone());
VerifyOrQuit(iterator.GetOption()->GetNumber() == 65535);
SuccessOrQuit(iterator.Advance());
VerifyOrQuit(iterator.IsDone());
VerifyOrQuit(iterator.GetOption() == nullptr);
message->Free();
testFreeInstance(instance);
}
#if OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
void TestReadBlockOptionValuesInvalidLength(void)
{
@@ -126,6 +208,8 @@ void TestReadBlockOptionValuesInvalidLength(void)
int main(void)
{
ot::TestCoapOverflow();
ot::TestCoapOptionNumberOverflow();
ot::TestCoapOptionNumberMaxRepeated();
#if OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
ot::TestReadBlockOptionValuesInvalidLength();
#endif