fix(nvhttp): use-after-free in clientpairingsecret crashes pair flow

[xdzleo]

Summary

clientpairingsecret() in src/nvhttp.cpp had a use-after-free in its success path that crashes Apollo with a deterministic 0xc0000005 access violation right after a successful pair handshake. The state file gets written with the new named_devices entry, then Sunshine.exe faults, and the client sees the connection drop and reports "pairing failed" even though the host actually paired.

This appears to be the root cause of #1436 and likely contributes to other reports of clients pairing repeatedly without ever succeeding (especially on iOS Moonlight, where I hit it 100% reproducibly with Darwin/25.5.0 over LAN against both v0.4.6 and current master dd99a822).

Root cause

map_id_sess is std::unordered_map<std::string, pair_session_t> — values, not pointers. The success branch did:

auto it = map_id_sess.find(client.uniqueID);
map_id_sess.erase(it);                 // destroys the pair_session_t
add_authorized_client(named_cert_p);   // save_state() + load_state() — heavy alloc, reuses freed slot
…
remove_session(sess);                  // sess.client.uniqueID -> corrupted memory -> 0xc0000005

add_authorized_client() runs save_state() (writes sunshine_state.json) and load_state() (reparses it, rebuilds client_root.named_devices, recreates the cert chain). That heap activity reliably reuses the freed pair_session_t slot, so the remove_session(sess) below dereferences a dangling reference. The crash is at the same offset every run because the allocator reuses the slot the same way.

The hash-mismatch branch never had this problem — it only calls remove_session(sess) once.

Fix

Drop the eager erase. remove_session(sess) at the end already handles both branches, and sess is still valid at that point because nothing in between mutates map_id_sess.

-      auto it = map_id_sess.find(client.uniqueID);
-      map_id_sess.erase(it);
-
       add_authorized_client(named_cert_p);
     } else {
       tree.put("root.paired", 0);
       BOOST_LOG(warning) << "Pair attempt failed due to same_hash: " << same_hash << ", verify: " << verify;
     }
 
+    // (extended comment in the diff explaining the prior UAF)
     remove_session(sess);

Net: +9 / -3 lines including the explanatory comment.

Test plan

• Reproduced the crash on master dd99a822 with iOS Moonlight (Darwin/25.5.0) on LAN — Application Error 0xc0000005 at fixed offset right after the /pair?phrase=clientpairingsecret request, sunshine_state.json already updated with the new device.
• Same repro on v0.4.6 release (different binary offset, same call stack).
• After this patch: pair completes cleanly, no application errors, client transitions to paired state, streaming works.
• Hash-mismatch path still cleans up the session (verified by injecting a wrong PIN — remove_session(sess) is hit once, no leak).
• No new warnings under -Wall -Wextra (gcc 16, ucrt64).

Notes for reviewers

While auditing this path I noticed two adjacent issues that I'm happy to send as separate PRs if you're interested:

1. map_id_sess and client_root are mutated from multiple threads (HTTPS pair handler, HTTP pair handler, web-UI pin() thread) with no lock. The same UAF class is reachable concurrently — two simultaneous pair attempts can race the destructor.
2. nvhttp::pin() uses std::begin(map_id_sess)->second to pick a session, which doesn't correlate the user-typed PIN with the specific pending device. With two concurrent pair attempts the wrong device can be paired with the legitimate user's PIN.

Both are pre-existing but worth fixing while pair-flow ownership is in mind. Happy to file follow-ups.

[ClassicOldSong]

Thanks but I'm not accepting commits from Claude. You can do a review yourself and commit with your own email address. For example, those comments are pure noises for future maintenance because the problematic code is already gone.