Skip to content

Client: use-after-free crash when a join is refused after the asset download completes #315

Closed

Description

@Kheartz

Summary

When the client refuses a join from inside Instance::OnAssetsDownloaded (any refuse path: StartAllResources() failing, package verification failing, etc.), it crashes about 100 ms later. The crash is an access violation in MafiaNet's FileListTransfer::DecodeSetHeader (or DecodeFile), on the main thread. When the freed memory hasn't been reused yet, it's a double free instead, and the process dies with 0xC0000374. That variant bypasses UE's crash handler, so it leaves no Saved\Crashes folder.

Repro

  1. Run a server whose client resources fail to start, for example one where a client resource declares a dependency that isn't shipped to clients.
  2. Join it.
  3. The log ends with Failed to start client resources; refusing to join → Disconnecting from the server... → Client scripting module reset, and then the process crashes.

FrameworkTests repro: the download_disconnect module in #316.

Root cause

  1. FileListTransfer::DecodeSetHeader calls AssetDownloadFileProgress::OnDownloadComplete.
  2. That calls Instance::OnAssetsDownloaded synchronously.
  3. The refuse path calls net->Disconnect(), which runs RakPeer::Shutdown() → FileListTransfer::OnRakPeerShutdown() → Clear(). This deletes every FileListReceiver, including the one DecodeSetHeader is still using.
  4. OnDownloadComplete returns false, so MafiaNet calls fileListReceiver->downloadHandler->OnDereference() on the freed receiver (FileListTransfer.cpp:332-338, MafiaNet 0.22.1).
  5. If the freed memory hasn't been reused yet, the following fileListReceivers.Delete / OP_DELETE frees it a second time instead.

DecodeFile (FileListTransfer.cpp:454-461) has the same tail. This became reachable with 449b237 ("Client: Refuse a join whose resources fail to start"). It's confirmed from a release minidump on 33.0.1.

Wider than the download path: RakPeer::Shutdown() also clears packetAllocationPool, including the packet being dispatched, and RakPeer::Receive deallocates it after the plugin returns. So any Disconnect() made while NetworkClient::Update() is dispatching is unsafe: from a plugin callback, an RPC handler, or a project's own code reached from either.

Fix

NetworkClient::Disconnect() called during dispatch defers the peer shutdown until dispatch unwinds (#316). That covers every caller, so OnAssetsDownloaded doesn't need to change.

Related (not in the fix PR)

AssetFileTransfer::OnClosedConnection (networking/network_client.cpp) doesn't call FileListTransfer::OnClosedConnection, so RemoveReceiver() never runs when a connection drops. Receivers are only freed later, by Shutdown() → Clear().

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

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions