Skip to content

Commit

Permalink
Add missing null checks on PacketBufferHandle::New calls (#22274)
Browse files Browse the repository at this point in the history
- Some callers of PacketBufferHandle::New did not null-check on
  failure to allocate. This is strongly linked to some crashes

Fixes #22262

This PR:
- Adds missing null checks required by API contract

Testing done:
- Unit tests still pass
- Conditions under which a crash previously occured no longer see
  a crash occur in manual testing against a real DUT
  • Loading branch information
tcarmelveilleux authored and pull[bot] committed Feb 6, 2024
1 parent 483ebd9 commit 1964816
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 3 deletions.
4 changes: 3 additions & 1 deletion examples/chip-tool/commands/pairing/CloseSessionCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ CHIP_ERROR CloseSessionCommand::CloseSession(Messaging::ExchangeManager & exchan
SecureChannel::kProtocolCodeCloseSession);

size_t reportSize = statusReport.Size();
Encoding::LittleEndian::PacketBufferWriter bbuf(MessagePacketBuffer::New(reportSize), reportSize);
auto packetBuffer = MessagePacketBuffer::New(reportSize);
VerifyOrReturnError(!packetBuffer.IsNull(), CHIP_ERROR_NO_MEMORY);
Encoding::LittleEndian::PacketBufferWriter bbuf(std::move(packetBuffer), reportSize);
statusReport.WriteToBuffer(bbuf);

System::PacketBufferHandle msg = bbuf.Finalize();
Expand Down
1 change: 1 addition & 0 deletions src/app/BufferedReadCallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ CHIP_ERROR BufferedReadCallback::BufferListItem(TLV::TLVReader & reader)
// we can improve this.
//
handle = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY);

writer.Init(std::move(handle), false);

Expand Down
1 change: 1 addition & 0 deletions src/app/ClusterStateCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ CHIP_ERROR ClusterStateCache::UpdateEventCache(const EventHeader & aEventHeader,
return CHIP_NO_ERROR;
}
System::PacketBufferHandle handle = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes);
VerifyOrReturnError(!handle.IsNull(), CHIP_ERROR_NO_MEMORY);

System::PacketBufferTLVWriter writer;
writer.Init(std::move(handle), false);
Expand Down
5 changes: 4 additions & 1 deletion src/protocols/secure_channel/PairingSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,10 @@ class DLL_EXPORT PairingSession : public SessionDelegate

Protocols::SecureChannel::StatusReport statusReport(generalCode, Protocols::SecureChannel::Id, protocolCode);

Encoding::LittleEndian::PacketBufferWriter bbuf(System::PacketBufferHandle::New(statusReport.Size()));
auto handle = System::PacketBufferHandle::New(statusReport.Size());
VerifyOrReturn(!handle.IsNull(), ChipLogError(SecureChannel, "Failed to allocate status report message"));
Encoding::LittleEndian::PacketBufferWriter bbuf(std::move(handle));

statusReport.WriteToBuffer(bbuf);

System::PacketBufferHandle msg = bbuf.Finalize();
Expand Down
4 changes: 3 additions & 1 deletion src/setup_payload/AdditionalDataPayloadGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ AdditionalDataPayloadGenerator::generateAdditionalDataPayload(AdditionalDataPayl
TLVWriter innerWriter;

// Initialize TLVWriter
writer.Init(chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize));
auto tempBuffer = chip::System::PacketBufferHandle::New(chip::System::PacketBuffer::kMaxSize);
VerifyOrReturnError(!tempBuffer.IsNull(), CHIP_ERROR_NO_MEMORY);
writer.Init(std::move(tempBuffer));

ReturnErrorOnFailure(writer.OpenContainer(AnonymousTag(), kTLVType_Structure, innerWriter));

Expand Down

0 comments on commit 1964816

Please sign in to comment.