diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp index 59e1e822901db0..a60999be2ff626 100644 --- a/src/system/SystemPacketBuffer.cpp +++ b/src/system/SystemPacketBuffer.cpp @@ -508,37 +508,56 @@ void PacketBuffer::AddRef() PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aReservedSize) { - // Adding three 16-bit-int sized numbers together will never overflow - // assuming int is at least 32 bits. + // Ensure that SIZE_MAX is at least 32 bits static_assert(INT_MAX >= INT32_MAX, "int is not big enough"); + static_assert(SIZE_MAX >= INT_MAX, "Our additions may not fit in size_t"); + + // Sanity check for kStructureSize and putting an upper bound. static_assert(PacketBuffer::kStructureSize == sizeof(PacketBuffer), "PacketBuffer size mismatch"); - static_assert(PacketBuffer::kStructureSize < UINT16_MAX, "Check for overflow more carefully"); - static_assert(SIZE_MAX >= INT_MAX, "Our additions might not fit in size_t"); - static_assert(PacketBuffer::kMaxSizeWithoutReserve <= UINT32_MAX, "PacketBuffer may have size not fitting uint32_t"); + static_assert(PacketBuffer::kStructureSize < UINT16_MAX, "kStructureSize should not exceed UINT16_MAX"); + static_assert(PacketBuffer::kMaxSizeWithoutReserve <= UINT16_MAX, "PacketBuffer may have size not fitting uint16_t"); + + // Have a bounds check for aAvailableSize to ensure that + // the next overflow check is done correctly. + if (aAvailableSize > UINT32_MAX) + { + ChipLogError(chipSystemLayer, "PacketBuffer: AvailableSize of a buffer cannot exceed UINT32_MAX"); + return PacketBufferHandle(); + } + + // Cast all to uint64_t and add to consider the widest possible result. + uint64_t sumOfSizes = static_cast(aAvailableSize) + static_cast(aReservedSize) + + static_cast(PacketBuffer::kStructureSize); + uint64_t sumOfAvailAndReserved = static_cast(aAvailableSize) + static_cast(aReservedSize); + + // Ensure that the sum does not overflow a size_t whose max value is at + // least INT_MAX. + if (sumOfSizes > std::numeric_limits::max()) + { + ChipLogError(chipSystemLayer, "PacketBuffer: Sizes of allocation request are invalid"); + return PacketBufferHandle(); + } + #if CHIP_SYSTEM_CONFIG_USE_LWIP // LwIP based APIs have a maximum buffer size of UINT16_MAX. Ensure that // limit is met during allocation. - VerifyOrDieWithMsg(aAvailableSize + aReservedSize < UINT16_MAX, chipSystemLayer, - "LwIP based systems can handle only up to UINT16_MAX!"); + VerifyOrDieWithMsg(sumOfAvailAndReserved < UINT16_MAX, chipSystemLayer, "LwIP based systems can handle only up to UINT16_MAX!"); #endif // CHIP_SYSTEM_CONFIG_USE_LWIP - // When `aAvailableSize` fits in uint16_t (as tested below) and size_t is at least 32 bits (as asserted above), - // these additions will not overflow. - const size_t lAllocSize = aReservedSize + aAvailableSize; - const size_t lBlockSize = PacketBuffer::kStructureSize + lAllocSize; + // The above check against the size_t limit ensures that the below sum will not + // overflow. + const size_t lAllocSize = static_cast(sumOfAvailAndReserved); PacketBuffer * lPacket; CHIP_SYSTEM_FAULT_INJECT(FaultInjection::kFault_PacketBufferNew, return PacketBufferHandle()); - // TODO: Change the max to a lower value - if (aAvailableSize > UINT32_MAX || lAllocSize > PacketBuffer::kMaxSizeWithoutReserve || lBlockSize > UINT32_MAX) + if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve) { - ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large."); + ChipLogError(chipSystemLayer, "PacketBuffer: allocation exceeding buffer capacity limits."); return PacketBufferHandle(); } #if CHIP_SYSTEM_CONFIG_USE_LWIP - lPacket = static_cast( pbuf_alloc(PBUF_RAW, static_cast(lAllocSize), CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE)); @@ -546,7 +565,6 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese #elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_POOL - static_cast(lBlockSize); #if !CHIP_SYSTEM_CONFIG_NO_LOCKING && CHIP_SYSTEM_CONFIG_FREERTOS_LOCKING if (!sBufferPoolMutex.isInitialized()) { @@ -565,7 +583,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese UNLOCK_BUF_POOL(); #elif CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP - + const size_t lBlockSize = PacketBuffer::kStructureSize + lAllocSize; lPacket = reinterpret_cast(chip::Platform::MemoryAlloc(lBlockSize)); SYSTEM_STATS_INCREMENT(chip::System::Stats::kSystemLayer_NumPacketBufs);