Skip to content

NetworkState: HTTP/WebSocket submission races cleanup (TOCTOU) causing UAF #1019

Description

Summary

NetworkState (Source/Global/NetworkState.cpp) does not effectively serialize network-operation
submission against the start of cleanup. When a client calls HCHttpCallPerformAsync or
HCWebSocketConnectAsync on one thread while another thread calls HCCleanup / HCCleanupAsync,
there is a time-of-check-to-time-of-use window that can result in the dereferencing of the
m_networkState pointer held by the global http_singleton and subsequent UAF in HttpCallPerformAsyncProvider/WebSocketConnectAsyncProvider.

Affected Code

  • Source/Global/global_publics.cpp — HCHttpCallPerformAsync
  • Source/WebSocket/websocket_publics.cpp — HCWebSocketConnectAsync
  • Source/Global/global.cpp — http_singleton::CleanupAsyncProvider
  • Source/Global/NetworkState.cpp — HttpCallPerformAsyncProvider,
    WebSocketConnectAsyncProvider, CleanupAsyncProvider

Background

This problem was previously observed in Issue 999 , and was partially addressed by PR 1000. While that work reduced the frequency of this issue, there are multiple races still in play here.

Race A - Request added after CleanupAsyncProvider starts while an active request is in progress

This race involves having one active request and one request rejected by the recently added check on NetworkState::m_cleanupStarted.

  1. Thread A: HttpCallPerformAsyncProvider::Begin inserts Request 1 into m_activeHttpRequests
  2. Thread B: CleanupAsyncProvider::Begin snapshots m_activeHttpRequests (includes Thread A's request)
  3. Thread A: HttpCallPerformAsyncProvider::Begin tries to insert Request 2, but is blocked by the lock and returns E_HC_NOT_INITIALISED. This failure schedules completion with the failed result.
  4. HttpCallPerformAsyncProvider::Cleanup is called for Request 1. Request 1 is the only entry in m_activeHttpRequests, so NetworkState::scheduleCleanup returns true and schedules the singleton's cleanup. Singleton cleanup triggers, and since there's nothing holding a singleton ref, releases the singleton, which then releases NetworkState.
  5. HttpCallPerformAsyncProvider::Cleanup is called for Request 2. The null NetworkState is dereferenced, resulting in a crash.

The ordering of steps 4 and 5 is nondeterministic when using a multithreaded task queue, and thus this issue is observed quite rarely.

Race B - Request added after CleanupAsyncProvider starts while no active requests are in progress

  1. Thread A starts NetworkState::CleanUpAsyncProvider with no HTTP requests currently tracked. It takes ownership of the mutex at line 525 and sets the NetworkState::m_cleanupStarted flag.
  2. NetworkState::scheduleCleanup evaluates to true, as there are no requests active. This result will lead to the scheduling of the continuation of the cleanup process at line 582.
  3. Between the mutex being released (line 555) and the time cleanup finishes, Thread B starts HttpCallPerformAsyncProvider::Begin. It reaches the early return at line 178, which results in the scheduling of HttpCallPerformAsyncProvider::Cleanup for the request.
  4. HttpCallPerformAsyncProvider::Cleanup may then fail in an assortment of ways.
    1. If step 2 has proceeded far enough to reach singleton cleanup, the NetworkState is dereferenced as a null pointer.
    2. The scheduling of the cleanup block may fail, as it has already been started by NetworkState::CleanUpAsyncProvider.

Impact

  • Intermittent crash (null dereference / UB) during shutdown when HTTP or WebSocket calls are
    submitted concurrently with HCCleanup.
  • Timing-dependent; more likely under load or with frequent init/cleanup cycles.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions