mirror of
https://github.com/espressif/openthread.git
synced 2026-08-13 14:17:47 +00:00
[trel] fix possible use-after-free in HandleTxtResult() (#11781)
This change fixes a potential use-after-free issue in the `PeerDiscoverer::HandleTxtResult()` method. When processing a TXT record, the corresponding `Peer` object could be removed if it was identified as the device itself. However, a subsequent call to `UpdatePeerState()` would still use the dangling reference to the removed `Peer` object. The fix merges the logic from the now-removed `ProcessPeerTxtData()` method directly into `HandleTxtResult()`. After a `Peer` is removed, the local `peer` pointer is set to `nullptr`, and the call to `UpdatePeerState()` is guarded by a null check to prevent using the invalid pointer.
This commit is contained in:
@@ -444,27 +444,16 @@ void PeerDiscoverer::HandleTxtResult(otInstance *aInstance, const otPlatDnssdTxt
|
||||
|
||||
void PeerDiscoverer::HandleTxtResult(const Dnssd::TxtResult &aResult)
|
||||
{
|
||||
Peer *peer;
|
||||
Peer *peer = nullptr;
|
||||
TxtData txtData;
|
||||
TxtData::Info txtInfo;
|
||||
|
||||
VerifyOrExit(IsRunning());
|
||||
|
||||
peer = Get<PeerTable>().FindMatching(Peer::ServiceNameMatcher(aResult.mServiceInstance));
|
||||
VerifyOrExit(peer != nullptr);
|
||||
|
||||
ProcessPeerTxtData(aResult, *peer);
|
||||
|
||||
UpdatePeerState(*peer);
|
||||
|
||||
exit:
|
||||
return;
|
||||
}
|
||||
|
||||
void PeerDiscoverer::ProcessPeerTxtData(const Dnssd::TxtResult &aResult, Peer &aPeer)
|
||||
{
|
||||
TxtData txtData;
|
||||
TxtData::Info txtInfo;
|
||||
|
||||
aPeer.mTxtDataValidated = false;
|
||||
peer->mTxtDataValidated = false;
|
||||
|
||||
VerifyOrExit(aResult.mTtl != 0);
|
||||
|
||||
@@ -474,14 +463,15 @@ void PeerDiscoverer::ProcessPeerTxtData(const Dnssd::TxtResult &aResult, Peer &a
|
||||
|
||||
if (txtInfo.mExtAddress == Get<Mac::Mac>().GetExtAddress())
|
||||
{
|
||||
LogInfo("Peer %s is this device itself", aPeer.mServiceName.AsCString());
|
||||
Get<PeerTable>().RemoveMatching(aPeer);
|
||||
LogInfo("Peer %s is this device itself", peer->mServiceName.AsCString());
|
||||
Get<PeerTable>().RemoveMatching(*peer);
|
||||
peer = nullptr;
|
||||
ExitNow();
|
||||
}
|
||||
|
||||
aPeer.SetExtPanId(txtInfo.mExtPanId);
|
||||
peer->SetExtPanId(txtInfo.mExtPanId);
|
||||
|
||||
if (aPeer.GetExtAddress() != txtInfo.mExtAddress)
|
||||
if (peer->GetExtAddress() != txtInfo.mExtAddress)
|
||||
{
|
||||
// Remove any peer that is associated with the same ExtAddress.
|
||||
// These are likely stale entries. This ensure we have at most
|
||||
@@ -489,13 +479,16 @@ void PeerDiscoverer::ProcessPeerTxtData(const Dnssd::TxtResult &aResult, Peer &a
|
||||
|
||||
Get<PeerTable>().RemoveAndFreeAllMatching(txtInfo.mExtAddress);
|
||||
|
||||
aPeer.SetExtAddress(txtInfo.mExtAddress);
|
||||
peer->SetExtAddress(txtInfo.mExtAddress);
|
||||
}
|
||||
|
||||
aPeer.mTxtDataValidated = true;
|
||||
peer->mTxtDataValidated = true;
|
||||
|
||||
exit:
|
||||
return;
|
||||
if (peer != nullptr)
|
||||
{
|
||||
UpdatePeerState(*peer);
|
||||
}
|
||||
}
|
||||
|
||||
void PeerDiscoverer::StartHostAddressResolver(Peer &aPeer)
|
||||
|
||||
@@ -252,7 +252,6 @@ private:
|
||||
void StopServiceResolvers(Peer &aPeer);
|
||||
void HandleSrvResult(const Dnssd::SrvResult &aResult);
|
||||
void HandleTxtResult(const Dnssd::TxtResult &aResult);
|
||||
void ProcessPeerTxtData(const Dnssd::TxtResult &aResult, Peer &aPeer);
|
||||
void StartHostAddressResolver(Peer &aPeer);
|
||||
void StopHostAddressResolver(Peer &aPeer);
|
||||
void HandleAddressResult(const Dnssd::AddressResult &aResult);
|
||||
|
||||
Reference in New Issue
Block a user