[message] check for potential integer overflows (#11834)

This commit adds checks to prevent potential integer overflow issues
within the `Message` class.

Previously, calculations involving message offset and length, such as
`offset + length`, assumed the caller would provide values within a
safe range. However, in some edge cases where larger values are
given, this addition could wrap around. This could lead to incorrect
behavior, potential memory corruption, or assertion failures.

To address this, this change introduces a new generic utility
function, `CanAddSafely()`, to detect unsigned integer addition
overflows. This check is now applied in the following `Message`
methods to validate lengths and offsets before performing
arithmetic:

- `AppendBytes()`: Returns an error if `offset + length` overflows.
- `AppendBytesFromMessage()`: Returns an error on overflow.
- `GetFirstChunk()`: Safely clamps the read length to the available
  message length.
- `WriteBytes()`: Asserts if `offset + length` overflows.

Unit tests for the new `CanAddSafely()` utility are included, covering
`uint8_t` and `uint16_t` cases.
This commit is contained in:
Abtin Keshavarzian
2025-08-20 22:22:39 -07:00
committed by GitHub
parent 364bbf5e49
commit 968dbb2a04
3 changed files with 61 additions and 7 deletions
+16 -7
View File
@@ -263,12 +263,14 @@ void Message::Free(void)
Error Message::SetLength(uint16_t aLength)
{
Error error = kErrorNone;
uint16_t totalLengthRequest = GetReserved() + aLength;
Error error;
uint16_t size;
VerifyOrExit(totalLengthRequest >= GetReserved(), error = kErrorInvalidArgs);
VerifyOrExit(CanAddSafely<uint16_t>(GetReserved(), aLength), error = kErrorNoBufs);
size = GetReserved() + aLength;
SuccessOrExit(error = ResizeMessage(size));
SuccessOrExit(error = ResizeMessage(totalLengthRequest));
GetMetadata().mLength = aLength;
// Correct the offset in case shorter length is set.
@@ -368,10 +370,12 @@ void Message::InvokeTxCallback(Error aError)
Error Message::AppendBytes(const void *aBuf, uint16_t aLength)
{
Error error = kErrorNone;
Error error;
uint16_t oldLength = GetLength();
SuccessOrExit(error = SetLength(GetLength() + aLength));
VerifyOrExit(CanAddSafely<uint16_t>(oldLength, aLength), error = kErrorNoBufs);
SuccessOrExit(error = SetLength(oldLength + aLength));
WriteBytes(oldLength, aBuf, aLength);
exit:
@@ -389,7 +393,11 @@ Error Message::AppendBytesFromMessage(const Message &aMessage, uint16_t aOffset,
uint16_t writeOffset = GetLength();
Chunk chunk;
VerifyOrExit(CanAddSafely<uint16_t>(aOffset, aLength), error = kErrorInvalidArgs);
VerifyOrExit(aMessage.GetLength() >= aOffset + aLength, error = kErrorParse);
VerifyOrExit(CanAddSafely<uint16_t>(GetLength(), aLength), error = kErrorNoBufs);
SuccessOrExit(error = SetLength(GetLength() + aLength));
aMessage.GetFirstChunk(aOffset, aLength, chunk);
@@ -531,7 +539,7 @@ void Message::GetFirstChunk(uint16_t aOffset, uint16_t &aLength, Chunk &aChunk)
VerifyOrExit(aOffset < GetLength(), aChunk.SetLength(0));
if (aOffset + aLength >= GetLength())
if (!CanAddSafely<uint16_t>(aOffset, aLength) || (aOffset + aLength >= GetLength()))
{
aLength = GetLength() - aOffset;
}
@@ -700,6 +708,7 @@ void Message::WriteBytes(uint16_t aOffset, const void *aBuf, uint16_t aLength)
const uint8_t *bufPtr = reinterpret_cast<const uint8_t *>(aBuf);
MutableChunk chunk;
OT_ASSERT(CanAddSafely<uint16_t>(aOffset, aLength));
OT_ASSERT(aOffset + aLength <= GetLength());
GetFirstChunk(aOffset, aLength, chunk);
+20
View File
@@ -145,6 +145,26 @@ template <typename IntType> int8_t ClampToInt8(IntType aValue)
static_cast<IntType>(NumericLimits<int8_t>::kMax)));
}
/**
* Indicates whether or not the addition of two unsigned integers will result in an overflow.
*
* @tparam UintType The value type (MUST be `uint8_t`, `uint16_t`, `uint32_t`, or `uint64_t`).
*
* @param[in] aFirstValue The first operand in the addition.
* @param[in] aSecondValue The second operand in the addition.
*
* @retval TRUE If the addition can be performed safely and does not cause an overflow.
* @retval FALSE If the addition will result in an overflow.
*/
template <typename UintType> bool CanAddSafely(UintType aFirstValue, UintType aSecondValue)
{
static_assert(TypeTraits::IsUint<UintType>::kValue, "UintType must be an unsigned int (8, 16, 32, or 64 bit len)");
UintType sum = static_cast<UintType>(aFirstValue + aSecondValue);
return (sum >= aFirstValue);
}
/**
* This template function checks whether a given value is in a given closed range [min, max].
*
+25
View File
@@ -110,6 +110,31 @@ void TestNumUtils(void)
u32 = 0xfff0000;
VerifyOrQuit(ClampToUint16(u32) == 0xffff);
VerifyOrQuit(CanAddSafely<uint8_t>(0, 0));
VerifyOrQuit(CanAddSafely<uint8_t>(100, 0));
VerifyOrQuit(CanAddSafely<uint8_t>(0, 100));
VerifyOrQuit(CanAddSafely<uint8_t>(200, 55));
VerifyOrQuit(CanAddSafely<uint8_t>(56, 199));
VerifyOrQuit(CanAddSafely<uint8_t>(127, 127));
VerifyOrQuit(!CanAddSafely<uint8_t>(200, 56));
VerifyOrQuit(!CanAddSafely<uint8_t>(100, 156));
VerifyOrQuit(!CanAddSafely<uint8_t>(1, 255));
VerifyOrQuit(!CanAddSafely<uint8_t>(255, 1));
VerifyOrQuit(!CanAddSafely<uint8_t>(255, 255));
VerifyOrQuit(!CanAddSafely<uint8_t>(128, 128));
VerifyOrQuit(CanAddSafely<uint16_t>(0, 0));
VerifyOrQuit(CanAddSafely<uint16_t>(0xffff, 0));
VerifyOrQuit(CanAddSafely<uint16_t>(0, 0xffff));
VerifyOrQuit(CanAddSafely<uint16_t>(0xff00, 0xff));
VerifyOrQuit(CanAddSafely<uint16_t>(0xfff, 0xf000));
VerifyOrQuit(!CanAddSafely<uint16_t>(0xffff, 1));
VerifyOrQuit(!CanAddSafely<uint16_t>(1, 0xffff));
VerifyOrQuit(!CanAddSafely<uint16_t>(65000, 65000));
VerifyOrQuit(!CanAddSafely<uint16_t>(32768, 32768));
VerifyOrQuit(IsValueInRange<uint8_t>(5, 5, 10));
VerifyOrQuit(IsValueInRange<uint8_t>(7, 5, 10));
VerifyOrQuit(IsValueInRange<uint8_t>(10, 5, 10));