Skip to content
This repository was archived by the owner on Dec 20, 2023. It is now read-only.

Commit 55e293c

Browse files
emargolisrobszewczyk
authored andcommitted
Clear PacketBuffer Content When Freed.
-- Added new PacketBuffer::Clear() method to wipe the contents of buffers as they are returned to the pool. -- Also eliminated the integer overflow vulnerability by increasing the size (to uint32_t) of the rFrameLen parameter to the WeaveMessageLayer::DecodeMessageWithLength() method. This change addresses CVE security vulnerability: CVE-2019-5040
1 parent cffea1f commit 55e293c

File tree

5 files changed

+26
-6
lines changed

5 files changed

+26
-6
lines changed

src/lib/core/WeaveConnection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,7 +1394,7 @@ void WeaveConnection::HandleDataReceived(TCPEndPoint *endPoint, PacketBuffer *da
13941394
uint8_t* payload;
13951395
uint16_t payloadLen;
13961396
PacketBuffer* payloadBuf = NULL;
1397-
uint16_t frameLen;
1397+
uint32_t frameLen;
13981398

13991399
packetInfo.Clear();
14001400
con->GetPeerAddressInfo(packetInfo);
@@ -1808,7 +1808,7 @@ void WeaveConnection::HandleBleMessageReceived(BLEEndPoint *endPoint, PacketBuff
18081808
uint8_t *payload;
18091809
uint16_t payloadLen;
18101810
PacketBuffer *payloadBuf;
1811-
uint16_t frameLen;
1811+
uint32_t frameLen;
18121812
WEAVE_ERROR err;
18131813

18141814
// Initialize the message info structure.

src/lib/core/WeaveMessageLayer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,7 +1410,7 @@ WEAVE_ERROR WeaveMessageLayer::EncodeMessageWithLength(WeaveMessageInfo *msgInfo
14101410
}
14111411

14121412
WEAVE_ERROR WeaveMessageLayer::DecodeMessageWithLength(PacketBuffer *msgBuf, uint64_t sourceNodeId, WeaveConnection *con,
1413-
WeaveMessageInfo *msgInfo, uint8_t **rPayload, uint16_t *rPayloadLen, uint16_t *rFrameLen)
1413+
WeaveMessageInfo *msgInfo, uint8_t **rPayload, uint16_t *rPayloadLen, uint32_t *rFrameLen)
14141414
{
14151415
uint8_t *dataStart = msgBuf->Start();
14161416
uint16_t dataLen = msgBuf->DataLength();
@@ -1426,7 +1426,7 @@ WEAVE_ERROR WeaveMessageLayer::DecodeMessageWithLength(PacketBuffer *msgBuf, uin
14261426
uint16_t msgLen = LittleEndian::Get16(dataStart);
14271427

14281428
// The frame length is the length of the message plus the length of the length field.
1429-
*rFrameLen = msgLen + 2;
1429+
*rFrameLen = static_cast<uint32_t>(msgLen) + 2;
14301430

14311431
// Error if the message buffer doesn't contain the entire message, or is too
14321432
// long to ever fit in the buffer.

src/lib/core/WeaveMessageLayer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ class NL_DLL_EXPORT WeaveMessageLayer
733733
WEAVE_ERROR EncodeMessageWithLength(WeaveMessageInfo *msgInfo, PacketBuffer *msgBuf, WeaveConnection *con,
734734
uint16_t maxLen);
735735
WEAVE_ERROR DecodeMessageWithLength(PacketBuffer *msgBuf, uint64_t sourceNodeId, WeaveConnection *con,
736-
WeaveMessageInfo *msgInfo, uint8_t **rPayload, uint16_t *rPayloadLen, uint16_t *rFrameLen);
736+
WeaveMessageInfo *msgInfo, uint8_t **rPayload, uint16_t *rPayloadLen, uint32_t *rFrameLen);
737737

738738
static void HandleUDPMessage(UDPEndPoint *endPoint, PacketBuffer *msg, const IPPacketInfo *pktInfo);
739739
static void HandleUDPReceiveError(UDPEndPoint *endPoint, INET_ERROR err, const IPPacketInfo *pktInfo);

src/system/SystemPacketBuffer.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
#endif // WEAVE_SYSTEM_CONFIG_USE_LWIP
5151

5252
#include <Weave/Support/logging/WeaveLogging.h>
53+
#include <Weave/Support/crypto/WeaveCrypto.h>
5354
#include <Weave/Support/CodeUtils.h>
5455

5556
#include <SystemLayer/SystemStats.h>
@@ -583,6 +584,7 @@ void PacketBuffer::Free(PacketBuffer* aPacket)
583584
if (aPacket->ref == 0)
584585
{
585586
SYSTEM_STATS_DECREMENT(nl::Weave::System::Stats::kSystemLayer_NumPacketBufs);
587+
aPacket->Clear();
586588
#if WEAVE_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC
587589
aPacket->next = sFreeList;
588590
sFreeList = aPacket;
@@ -602,6 +604,21 @@ void PacketBuffer::Free(PacketBuffer* aPacket)
602604
#endif // !WEAVE_SYSTEM_CONFIG_USE_LWIP
603605
}
604606

607+
/**
608+
* Clear content of the packet buffer.
609+
*
610+
* This method is called by Free(), before the buffer is released to the free buffer pool.
611+
*/
612+
void PacketBuffer::Clear(void)
613+
{
614+
nl::Weave::Crypto::ClearSecretData(reinterpret_cast<uint8_t*>(this) + WEAVE_SYSTEM_PACKETBUFFER_HEADER_SIZE, this->AllocSize());
615+
tot_len = 0;
616+
len = 0;
617+
#if WEAVE_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC == 0
618+
alloc_size = 0;
619+
#endif // WEAVE_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC == 0
620+
}
621+
605622
/**
606623
* Free the first buffer in a chain, returning a pointer to the remaining buffers.
607624
`*

src/system/SystemPacketBuffer.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ class NL_DLL_EXPORT PacketBuffer : private pbuf
139139

140140
static PacketBuffer* BuildFreeList(void);
141141
#endif // !WEAVE_SYSTEM_CONFIG_USE_LWIP && WEAVE_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC
142+
143+
void Clear(void);
142144
};
143145

144146
} // namespace System
@@ -203,7 +205,8 @@ typedef union
203205
#endif // !WEAVE_SYSTEM_CONFIG_USE_LWIP && WEAVE_SYSTEM_CONFIG_PACKETBUFFER_MAXALLOC
204206

205207
/**
206-
* Return the size of the allocation including the PacketBuffer structure and its payload data space.
208+
* Return the size of the allocation including the reserved and payload data spaces but not including space
209+
* allocated for the PacketBuffer structure.
207210
*
208211
* @note The allocation size is equal or greater than \c aAllocSize paramater to \c Create method).
209212
*

0 commit comments

Comments
 (0)