GameNetworkingSockets icon indicating copy to clipboard operation
GameNetworkingSockets copied to clipboard

[Verified Bug - Crash - Latest Master] CSteamNetworkingICESession::STUNRequestCallback_ServerReflexiveCandidate

Open Jannes1000Orks opened this issue 1 year ago • 1 comments

Hey,

I hope this way of reporting a bug is fine!

STUNRequestCallback_ServerReflexiveCandidate Tests through all of the STUN Servers if the requests fail. I was able to spot 2 bugs that occur when STUN requests fail. This is the relevant Code bit starting in: steamnetworkingsockets_stun.cpp:1712

    // So we timed out to this STUN server
    const int nSTUNServerIdx = index_of( m_vecSTUNServers, info.m_pRequest->m_remoteAddr );
    CSharedSocket *pSharedSock = FindSharedSocketForCandidate( localAddr );
    if ( pSharedSock == nullptr || nSTUNServerIdx < 0 )
    {   // Just store an IPv6 all zeros to flag an invalid server reflexive candidate.
        bindResult.Clear();    
        ICECandidate *pCand = push_back_get_ptr( m_vecCandidates, ICECandidate( kICECandidateType_ServerReflexive, bindResult, localAddr, info.m_pRequest->m_remoteAddr ) );
        pCand->m_nPriority = 0;
        return;        
    }

    // Try the next server
    CSteamNetworkingSocketsSTUNRequest *pNewRequest = CSteamNetworkingSocketsSTUNRequest::SendBindRequest( pSharedSock, m_vecSTUNServers[nSTUNServerIdx+1], CRecvSTUNPktCallback( StaticSTUNRequestCallback_ServerReflexiveCandidate, this ), m_nEncoding );

Sometimes multiple different domain names can resolve to the same IP - in this case duplicate IP/port pairs can be in m_vecCandidates. index_of( m_vecSTUNServers, info.m_pRequest->m_remoteAddr ); Returns a previous index and an infinite loop of Stun requests is caused.

The second issue is here: CSteamNetworkingSocketsSTUNRequest::SendBindRequest( pSharedSock, m_vecSTUNServers[nSTUNServerIdx+1], CRecvSTUNPktCallback( StaticSTUNRequestCallback_ServerReflexiveCandidate, this ), m_nEncoding );

When sending the next bind request the bounds of m_vecSTUNServers are never checked, causing a crash.

Jannes1000Orks avatar May 27 '24 20:05 Jannes1000Orks

Thanks for this report.

zpostfacto avatar Aug 30 '24 20:08 zpostfacto