admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix use after free in UdpPort

Browse Source 
SendStunBindingRequest can cause a SocketAddress pointed to by the
iterator to be deleted asynchronously before returning, causing `it`
to be invalid when incrementing in the continuation step.

Bug: webrtc:7309,  webrtc:14131
Change-Id: I3f7d3d7c12935d9592ef3642679a821c58826df9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270744
Commit-Queue: Sameer Vijaykar <samvi@google.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37741}
This commit is contained in:
Sameer Vijaykar 2022-08-10 22:48:03 +02:00 committed by WebRTC LUCI CQ
parent 6af2ac41ac
commit 1e688612cc
3 changed files with 12 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
p2p/base/stun_port.cc
View File

@ -424,8 +424,13 @@ void UDPPort::SendStunBindingRequests() {
RTC_DCHECK(request_manager_.empty());
for (ServerAddresses::const_iterator it = server_addresses_.begin();
it != server_addresses_.end(); ++it) {
SendStunBindingRequest(*it);
it != server_addresses_.end();) {
// sending a STUN binding request may cause the current SocketAddress to be
// erased from the set, invalidating the loop iterator before it is
// incremented (even if the SocketAddress itself still exists). So make a
// copy of the loop iterator, which may be safely invalidated.
ServerAddresses::const_iterator addr = it++;
SendStunBindingRequest(*addr);
}
}

3
p2p/base/stun_port.h
View File

@ -210,6 +210,9 @@ class UDPPort : public Port {
void ResolveStunAddress(const rtc::SocketAddress& stun_addr);
void OnResolveResult(const rtc::SocketAddress& input, int error);
// Send a STUN binding request to the given address. Calling this method may
// cause the set of known server addresses to be modified, eg. by replacing an
// unresolved server address with a resolved address.
void SendStunBindingRequest(const rtc::SocketAddress& stun_addr);
// Below methods handles binding request responses.

11
p2p/base/stun_port_unittest.cc
View File

@ -307,13 +307,7 @@ class StunPortWithMockDnsResolverTest : public StunPortTest {
};
// Test that we can get an address from a STUN server specified by a hostname.
// Crashes on Linux, see webrtc:7416
#if defined(WEBRTC_LINUX) || defined(WEBRTC_WIN) || defined(WEBRTC_MAC)
#define MAYBE_TestPrepareAddressHostname DISABLED_TestPrepareAddressHostname
#else
#define MAYBE_TestPrepareAddressHostname TestPrepareAddressHostname
#endif
TEST_F(StunPortWithMockDnsResolverTest, MAYBE_TestPrepareAddressHostname) {
TEST_F(StunPortWithMockDnsResolverTest, TestPrepareAddressHostname) {
SetDnsResolverExpectations(
[](webrtc::MockAsyncDnsResolver* resolver,
webrtc::MockAsyncDnsResolverResult* resolver_result) {
@ -625,8 +619,7 @@ class StunIPv6PortTestWithMockDnsResolver : public StunIPv6PortTest {
};
// Test that we can get an address from a STUN server specified by a hostname.
// Crashes on Linux, see webrtc:7416
TEST_F(StunIPv6PortTestWithMockDnsResolver, MAYBE_TestPrepareAddressHostname) {
TEST_F(StunIPv6PortTestWithMockDnsResolver, TestPrepareAddressHostname) {
SetDnsResolverExpectations(
[](webrtc::MockAsyncDnsResolver* resolver,
webrtc::MockAsyncDnsResolverResult* resolver_result) {