From 9cb6c1b8f571a99fc56030b62cb555739e8920f7 Mon Sep 17 00:00:00 2001 From: Sean McClain Date: Tue, 6 Sep 2022 15:24:57 -0500 Subject: [PATCH 1/6] Add IsNull check to UDP Sockets SendMsgImpl prior to ptr dereference. --- src/inet/UDPEndPointImplSockets.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/inet/UDPEndPointImplSockets.cpp b/src/inet/UDPEndPointImplSockets.cpp index 131e20d9709b23..eefcbb5e6c8dc8 100644 --- a/src/inet/UDPEndPointImplSockets.cpp +++ b/src/inet/UDPEndPointImplSockets.cpp @@ -281,6 +281,9 @@ CHIP_ERROR UDPEndPointImplSockets::SendMsgImpl(const IPPacketInfo * aPktInfo, Sy // Ensure the destination address type is compatible with the endpoint address type. VerifyOrReturnError(mAddrType == aPktInfo->DestAddress.Type(), CHIP_ERROR_INVALID_ARGUMENT); + // Ensure packet buffer is not null + VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY); + // For now the entire message must fit within a single buffer. VerifyOrReturnError(!msg->HasChainedBuffer(), CHIP_ERROR_MESSAGE_TOO_LONG); From 7b6c89194c326abb286dae85de193debf2242826 Mon Sep 17 00:00:00 2001 From: Sean <97990350+s-mcclain@users.noreply.github.com> Date: Wed, 7 Sep 2022 09:01:11 -0500 Subject: [PATCH 2/6] Update UDPEndPointImplSockets.cpp Moved msg.IsNull check to beginning of SendMsgImpl --- src/inet/UDPEndPointImplSockets.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/inet/UDPEndPointImplSockets.cpp b/src/inet/UDPEndPointImplSockets.cpp index eefcbb5e6c8dc8..282b7993301779 100644 --- a/src/inet/UDPEndPointImplSockets.cpp +++ b/src/inet/UDPEndPointImplSockets.cpp @@ -274,6 +274,9 @@ CHIP_ERROR UDPEndPointImplSockets::ListenImpl() CHIP_ERROR UDPEndPointImplSockets::SendMsgImpl(const IPPacketInfo * aPktInfo, System::PacketBufferHandle && msg) { + // Ensure packet buffer is not null + VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY); + // Make sure we have the appropriate type of socket based on the // destination address. ReturnErrorOnFailure(GetSocket(aPktInfo->DestAddress.Type())); @@ -281,9 +284,6 @@ CHIP_ERROR UDPEndPointImplSockets::SendMsgImpl(const IPPacketInfo * aPktInfo, Sy // Ensure the destination address type is compatible with the endpoint address type. VerifyOrReturnError(mAddrType == aPktInfo->DestAddress.Type(), CHIP_ERROR_INVALID_ARGUMENT); - // Ensure packet buffer is not null - VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY); - // For now the entire message must fit within a single buffer. VerifyOrReturnError(!msg->HasChainedBuffer(), CHIP_ERROR_MESSAGE_TOO_LONG); From bfe230f9ab2d4896092458b949a504cd5909cdb5 Mon Sep 17 00:00:00 2001 From: Sean McClain Date: Wed, 7 Sep 2022 14:28:54 -0500 Subject: [PATCH 3/6] Add null check to MinimalMdns Server when calling CloneData --- src/inet/UDPEndPointImplSockets.cpp | 2 +- src/lib/dnssd/minimal_mdns/Server.cpp | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/inet/UDPEndPointImplSockets.cpp b/src/inet/UDPEndPointImplSockets.cpp index 282b7993301779..cd12fe42fb64d3 100644 --- a/src/inet/UDPEndPointImplSockets.cpp +++ b/src/inet/UDPEndPointImplSockets.cpp @@ -275,7 +275,7 @@ CHIP_ERROR UDPEndPointImplSockets::ListenImpl() CHIP_ERROR UDPEndPointImplSockets::SendMsgImpl(const IPPacketInfo * aPktInfo, System::PacketBufferHandle && msg) { // Ensure packet buffer is not null - VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY); + VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_INVALID_ARGUMENT); // Make sure we have the appropriate type of socket based on the // destination address. diff --git a/src/lib/dnssd/minimal_mdns/Server.cpp b/src/lib/dnssd/minimal_mdns/Server.cpp index 1dc54bfa81e6d3..48d8b0ea7b5ae6 100644 --- a/src/lib/dnssd/minimal_mdns/Server.cpp +++ b/src/lib/dnssd/minimal_mdns/Server.cpp @@ -364,14 +364,19 @@ CHIP_ERROR ServerBase::BroadcastImpl(chip::System::PacketBufferHandle && data, u /// for sending via `CloneData` /// /// TODO: this wastes one copy of the data and that could be optimized away - if (info->mAddressType == chip::Inet::IPAddressType::kIPv6) + PacketBufferHandle tempBuf = data.CloneData(); + if (tempBuf.IsNull()) { + // Not enough memory available to clone pbuf + err = CHIP_ERROR_NO_MEMORY; + } + else if (info->mAddressType == chip::Inet::IPAddressType::kIPv6) { - err = udp->SendTo(mIpv6BroadcastAddress, port, data.CloneData(), udp->GetBoundInterface()); + err = udp->SendTo(mIpv6BroadcastAddress, port, tempBuf, udp->GetBoundInterface()); } #if INET_CONFIG_ENABLE_IPV4 else if (info->mAddressType == chip::Inet::IPAddressType::kIPv4) { - err = udp->SendTo(mIpv4BroadcastAddress, port, data.CloneData(), udp->GetBoundInterface()); + err = udp->SendTo(mIpv4BroadcastAddress, port, tempBuf, udp->GetBoundInterface()); } #endif else From ce3677c973bd1f9414686705e79c8ca607b55a4e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 12 Sep 2022 10:22:44 -0400 Subject: [PATCH 4/6] Restyle --- src/lib/dnssd/minimal_mdns/Server.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dnssd/minimal_mdns/Server.cpp b/src/lib/dnssd/minimal_mdns/Server.cpp index 48d8b0ea7b5ae6..fb6efdaff56b99 100644 --- a/src/lib/dnssd/minimal_mdns/Server.cpp +++ b/src/lib/dnssd/minimal_mdns/Server.cpp @@ -365,7 +365,8 @@ CHIP_ERROR ServerBase::BroadcastImpl(chip::System::PacketBufferHandle && data, u /// /// TODO: this wastes one copy of the data and that could be optimized away PacketBufferHandle tempBuf = data.CloneData(); - if (tempBuf.IsNull()) { + if (tempBuf.IsNull()) + { // Not enough memory available to clone pbuf err = CHIP_ERROR_NO_MEMORY; } From d1c9c3575d98ffd7752839ce7530de7e19b1ca23 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 12 Sep 2022 10:25:21 -0400 Subject: [PATCH 5/6] Add namespace for PacketBufferHandle --- src/lib/dnssd/minimal_mdns/Server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dnssd/minimal_mdns/Server.cpp b/src/lib/dnssd/minimal_mdns/Server.cpp index fb6efdaff56b99..9ec5914d7be237 100644 --- a/src/lib/dnssd/minimal_mdns/Server.cpp +++ b/src/lib/dnssd/minimal_mdns/Server.cpp @@ -364,7 +364,7 @@ CHIP_ERROR ServerBase::BroadcastImpl(chip::System::PacketBufferHandle && data, u /// for sending via `CloneData` /// /// TODO: this wastes one copy of the data and that could be optimized away - PacketBufferHandle tempBuf = data.CloneData(); + chip::System::PacketBufferHandle tempBuf = data.CloneData(); if (tempBuf.IsNull()) { // Not enough memory available to clone pbuf From 651b07f59ecfc640fb235262ad82950929d01bc4 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 12 Sep 2022 10:38:35 -0400 Subject: [PATCH 6/6] Use std::move for tmp buffer to allow for && arguments --- src/lib/dnssd/minimal_mdns/Server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/Server.cpp b/src/lib/dnssd/minimal_mdns/Server.cpp index 9ec5914d7be237..9db53bee0c4c6c 100644 --- a/src/lib/dnssd/minimal_mdns/Server.cpp +++ b/src/lib/dnssd/minimal_mdns/Server.cpp @@ -372,12 +372,12 @@ CHIP_ERROR ServerBase::BroadcastImpl(chip::System::PacketBufferHandle && data, u } else if (info->mAddressType == chip::Inet::IPAddressType::kIPv6) { - err = udp->SendTo(mIpv6BroadcastAddress, port, tempBuf, udp->GetBoundInterface()); + err = udp->SendTo(mIpv6BroadcastAddress, port, std::move(tempBuf), udp->GetBoundInterface()); } #if INET_CONFIG_ENABLE_IPV4 else if (info->mAddressType == chip::Inet::IPAddressType::kIPv4) { - err = udp->SendTo(mIpv4BroadcastAddress, port, tempBuf, udp->GetBoundInterface()); + err = udp->SendTo(mIpv4BroadcastAddress, port, std::move(tempBuf), udp->GetBoundInterface()); } #endif else