Skip to content

Commit

Permalink
Fix PASESession and CASESession use-after-free
Browse files Browse the repository at this point in the history
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 project-chip#13146
  • Loading branch information
msandstedt committed Dec 22, 2021
1 parent d163857 commit df055c5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/protocols/secure_channel/PASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit df055c5

Please sign in to comment.