Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Jannes1000Orks opened this issue May 27, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@Jannes1000Orks
Copy link

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.

@zpostfacto zpostfacto added the bug Something isn't working label Aug 30, 2024
@zpostfacto
Copy link
Contributor

Thanks for this report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants