From 285d508d17b0e88bf4d7577ccb958841809bcb50 Mon Sep 17 00:00:00 2001 From: Jerry Johns Date: Wed, 29 Jun 2022 20:16:50 -0700 Subject: [PATCH] Fix EC Delegate use-after-free in IM (#20141) If an error was encountered parsing the SubscribeResponse message, ReadClient::OnMessageReceived would just null-out the EC pointer but not the delegate pointer within the EC. This meant that when we got back to the exchange management layer after unwinding the stack, it attempted to call OnExchangeClosing on the delegate that had by then, been free'ed as part of cleaning up the ReadClient object. --- src/app/ReadClient.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 63a4b95eb96ff1..5c60e94986cd62 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -407,12 +407,14 @@ CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchange { VerifyOrExit(apExchangeContext == mpExchangeCtx, err = CHIP_ERROR_INCORRECT_STATE); err = ProcessSubscribeResponse(std::move(aPayload)); + SuccessOrExit(err); - // Forget the context as SUBSCRIBE RESPONSE is the last message in SUBSCRIBE transaction and - // ExchangeContext::HandleMessage automatically closes a context if no other messages need to - // be sent or received. + // + // Null out the delegate and context as SubscribeResponse is the last message the Subscribe transaction and + // the exchange layer will automatically close the exchange. + // + mpExchangeCtx->SetDelegate(nullptr); mpExchangeCtx = nullptr; - SuccessOrExit(err); } else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse)) {