[ip6] fix out-of-bounds access and length corruption in Prefix (#13537)

`Ip6::Prefix::SizeForLength()` returned uncapped byte sizes for
prefix bit lengths greater than 128. This resulted in out-of-bounds
memory access and state corruption:

- In `Prefix::Tidy()`, `byteLength = GetBytesSize()` returned values
  greater than 16. For lengths 129-136, masking `m8[byteLength - 1]`
  overwrote `mLength` itself (offset 16 in packed `otIp6Prefix`), and
  for lengths >= 137, wrote beyond the struct. Furthermore,
  `(byteLength * 8 - mLength)` underflowed `uint8_t`, leading to
  undefined bit-shift behavior.
- In `Prefix::ToString()`, `sizeInUint16` calculated from
  `GetBytesSize()` exceeded 8 words, causing `AppendHexWords()` to read
  past the 16-byte address.

This commit resolves these issues by:
- Capping `Prefix::SizeForLength()` to `Prefix::kMaxSize` (16 bytes).
- Adding `VerifyOrExit(mLength < kMaxLength)` in `Prefix::Tidy()`.
- Updating `PrefixTlv::IsValid()` to check
  `mPrefixLength <= Ip6::Prefix::kMaxLength`.
- Adding unit test `TestIp6PrefixInvalidLengths()` in
  `test_ip_address.cpp`.
This commit is contained in:
Jonathan Hui
2026-08-22 15:43:45 -07:00
committed by GitHub
parent cf24e583f5
commit f42bf5f2b9
4 changed files with 79 additions and 4 deletions
+10 -2
View File
@@ -97,8 +97,13 @@ bool Prefix::IsCoveredBy(const NetworkPrefix &aNetworkPrefix) const
void Prefix::Tidy(void)
{
uint8_t byteLength = GetBytesSize();
uint8_t lastByteBitMask = static_cast<uint8_t>(~(static_cast<uint8_t>(1 << (byteLength * 8 - mLength)) - 1));
uint8_t byteLength;
uint8_t lastByteBitMask;
VerifyOrExit(mLength < kMaxLength);
byteLength = GetBytesSize();
lastByteBitMask = static_cast<uint8_t>(0xffU << (byteLength * 8 - mLength));
if (byteLength != 0)
{
@@ -109,6 +114,9 @@ void Prefix::Tidy(void)
{
mPrefix.mFields.m8[i] = 0;
}
exit:
return;
}
bool Prefix::operator==(const Prefix &aOther) const
+2 -1
View File
@@ -43,6 +43,7 @@
#include "common/clearable.hpp"
#include "common/encoding.hpp"
#include "common/equatable.hpp"
#include "common/num_utils.hpp"
#include "common/string.hpp"
#include "mac/mac_types.hpp"
@@ -275,7 +276,7 @@ public:
*
* @returns The size (in bytes) of the prefix.
*/
static uint8_t SizeForLength(uint8_t aLength) { return BytesForBitSize(aLength); }
static uint8_t SizeForLength(uint8_t aLength) { return Min(BytesForBitSize(aLength), kMaxSize); }
/**
* Indicates whether or not a given prefix length is valid for use as a NAT64 prefix.
+1 -1
View File
@@ -620,7 +620,7 @@ public:
{
return ((GetLength() >= sizeof(*this) - sizeof(NetworkDataTlv)) &&
(GetLength() >= Ip6::Prefix::SizeForLength(mPrefixLength) + sizeof(*this) - sizeof(NetworkDataTlv)) &&
(Ip6::Prefix::SizeForLength(mPrefixLength) <= sizeof(Ip6::Address)));
(mPrefixLength <= Ip6::Prefix::kMaxLength));
}
/**
+66
View File
@@ -929,6 +929,71 @@ void TestIp4Cidr(void)
}
}
void TestIp6PrefixInvalidLengths(void)
{
static const uint8_t kInvalidLengths[] = {129, 130, 136, 137, 144, 200, 255};
static const uint8_t kRawPrefix[OT_IP6_ADDRESS_SIZE] = {
0x20, 0x01, 0x0d, 0xb8, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00, 0x04, 0x00, 0x05, 0x00, 0x06,
};
static const char kExpectedPrefixString[] = "2001:db8:1:2:3:4:5:6";
OT_TOOL_PACKED_BEGIN
struct
{
Ip6::Prefix mPrefix;
uint8_t mCanary[16];
} OT_TOOL_PACKED_END packedPrefix;
printf("\nTestIp6PrefixInvalidLengths()\n");
for (uint16_t len = 0; len <= 255; len++)
{
uint8_t length = static_cast<uint8_t>(len);
if (length <= Ip6::Prefix::kMaxLength)
{
VerifyOrQuit(Ip6::Prefix::SizeForLength(length) == (length + 7) / 8);
}
else
{
VerifyOrQuit(Ip6::Prefix::SizeForLength(length) == Ip6::Prefix::kMaxSize);
}
}
for (uint8_t invalidLen : kInvalidLengths)
{
char expectedString[Ip6::Prefix::kInfoStringSize];
memset(&packedPrefix, 0, sizeof(packedPrefix));
memset(packedPrefix.mCanary, 0xaa, sizeof(packedPrefix.mCanary));
packedPrefix.mPrefix.InitFrom(kRawPrefix, invalidLen);
VerifyOrQuit(packedPrefix.mPrefix.GetLength() == invalidLen);
VerifyOrQuit(!packedPrefix.mPrefix.IsValid());
VerifyOrQuit(packedPrefix.mPrefix.GetBytesSize() == Ip6::Prefix::kMaxSize);
for (uint8_t b : packedPrefix.mCanary)
{
VerifyOrQuit(b == 0xaa, "Prefix::InitFrom corrupted memory beyond prefix");
}
packedPrefix.mPrefix.Tidy();
VerifyOrQuit(packedPrefix.mPrefix.GetLength() == invalidLen, "Prefix::Tidy mutated invalid length");
VerifyOrQuit(memcmp(packedPrefix.mPrefix.GetBytes(), kRawPrefix, sizeof(kRawPrefix)) == 0,
"Prefix::Tidy corrupted prefix bytes");
for (uint8_t b : packedPrefix.mCanary)
{
VerifyOrQuit(b == 0xaa, "Prefix::Tidy wrote out of bounds");
}
snprintf(expectedString, sizeof(expectedString), "%s/%u", kExpectedPrefixString, invalidLen);
VerifyOrQuit(strcmp(packedPrefix.mPrefix.ToString().AsCString(), expectedString) == 0,
"Prefix::ToString() output mismatch for length > 128");
}
}
} // namespace ot
int main(void)
@@ -939,6 +1004,7 @@ int main(void)
ot::TestIp6PrefixFromString();
ot::TestIp6Prefix();
ot::TestIp6PrefixTidy();
ot::TestIp6PrefixInvalidLengths();
ot::TestIp4MappedIp6Address();
ot::TestIp4Ip6Translation();
ot::TestIp4Cidr();