From df055c5f422e9a2c4486db9c959b3d4485b6ea87 Mon Sep 17 00:00:00 2001 From: Michael Sandstedt Date: Sun, 19 Dec 2021 11:25:33 -0600 Subject: [PATCH] Fix PASESession and CASESession use-after-free PASESession and CASESession clear exchange contexts for which they are registered as delegate without closing them. This leads to a situation where exchanges can callback into these objects after they are freed. This commit fixes the problem by setting the exchange context delegates to nullptr before clearing the exchange contexts. Fixes #13146 --- src/protocols/secure_channel/CASESession.cpp | 3 +++ src/protocols/secure_channel/PASESession.cpp | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 8c71b193a2a173..651a6d41f76017 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -255,6 +255,7 @@ void CASESession::OnResponseTimeout(ExchangeContext * ec) mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT); // Null out mExchangeCtxt so that Clear() doesn't try closing it. The // exchange will handle that. + mExchangeCtxt->SetDelegate(nullptr); mExchangeCtxt = nullptr; Clear(); } @@ -684,6 +685,7 @@ CHIP_ERROR CASESession::HandleSigma2Resume(System::PacketBufferHandle && msg) mCASESessionEstablished = true; // Forget our exchange, as no additional messages are expected from the peer + mExchangeCtxt->SetDelegate(nullptr); mExchangeCtxt = nullptr; // Call delegate to indicate session establishment is successful @@ -1118,6 +1120,7 @@ CHIP_ERROR CASESession::HandleSigma3(System::PacketBufferHandle && msg) mCASESessionEstablished = true; // Forget our exchange, as no additional messages are expected from the peer + mExchangeCtxt->SetDelegate(nullptr); mExchangeCtxt = nullptr; // Call delegate to indicate session establishment is successful diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index aadda0f8fae41d..b117eb60061c6c 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -346,6 +346,7 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec) mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT); // Null out mExchangeCtxt so that Clear() doesn't try closing it. The // exchange will handle that. + mExchangeCtxt->SetDelegate(nullptr); mExchangeCtxt = nullptr; Clear(); } @@ -824,6 +825,7 @@ CHIP_ERROR PASESession::HandleMsg3(System::PacketBufferHandle && msg) mPairingComplete = true; // Forget our exchange, as no additional messages are expected from the peer + mExchangeCtxt->SetDelegate(nullptr); mExchangeCtxt = nullptr; // Call delegate to indicate pairing completion @@ -843,6 +845,7 @@ void PASESession::OnSuccessStatusReport() mPairingComplete = true; // Forget our exchange, as no additional messages are expected from the peer + mExchangeCtxt->SetDelegate(nullptr); mExchangeCtxt = nullptr; // Call delegate to indicate pairing completion