fix(coop): base-defense use-after-free on dangling temp_ufo

src/Geoscape/GeoscapeState.cpp

@@ -165,8 +165,27 @@ static std::vector<int> sharedAlertSoldierIds(const std::vector<Soldier*>& v)
 { std::vector<int> o; for (auto* s : v) if (s) o.push_back(s->getId()); return o; }
 
 
-// coop
-Ufo* temp_ufo = 0;
+// coop: base-defense parameters captured when an attacking UFO arms a coop base
+// defense (handleBaseDefense). The coop battle start is DEFERRED across a
+// host<->client handshake, during which the attacking Ufo is freed (confirmed:
+// the armed UFO is deleted before startCoopMission runs). Holding a raw Ufo*
+// across that gap was a use-after-free ("save crashes on month end"), so we
+// snapshot exactly the fields the deferred startCoopMission needs and never
+// dereference the (possibly freed) UFO later.
+namespace
+{
+struct CoopBaseDefense
+{
+	bool pending = false;   // a coop base defense is armed, not yet consumed
+	bool coop = false;      // ufo->getCoop() at arm time
+	double lon = 0.0;
+	double lat = 0.0;
+	int damagePercentage = 0;
+	std::string missionCustomDeploy;
+	std::string alienRace;
+};
+CoopBaseDefense g_coopBaseDefense;
+}
 
 /**
  * Initializes all the elements in the Geoscape screen.
@@ -564,30 +583,33 @@ GeoscapeState::~GeoscapeState()
 
 void GeoscapeState::startCoopMission()
 {
-
-	// coop
-	if (temp_ufo && temp_ufo->getCoop() == false && _game->getSavedGame() && _game->getSavedGame()->getSelectedBase())
+	// coop: consume the base-defense snapshot captured at arm time. NEVER
+	// dereference the attacking UFO here - it has been freed during the deferred
+	// host<->client handshake (see CoopBaseDefense above).
+	if (g_coopBaseDefense.pending && g_coopBaseDefense.coop == false
+		&& _game->getSavedGame() && _game->getSavedGame()->getSelectedBase())
 	{
+		g_coopBaseDefense.pending = false; // one-shot; never re-fire on a stale snapshot
 
 		// Get the shade and texture for the globe at the location of the base, using the ufo position
 		int texture, shade;
-		double baseLon = temp_ufo->getLongitude();
-		double baseLat = temp_ufo->getLatitude();
+		double baseLon = g_coopBaseDefense.lon;
+		double baseLat = g_coopBaseDefense.lat;
 		_globe->getPolygonTextureAndShade(baseLon, baseLat, &texture, &shade);
 
 		int ufoDamagePercentage = 0;
 		if (_game->getMod()->getLessAliensDuringBaseDefense())
 		{
-			ufoDamagePercentage = temp_ufo->getDamagePercentage();
+			ufoDamagePercentage = g_coopBaseDefense.damagePercentage;
 		}
 
 		SavedBattleGame* bgame = new SavedBattleGame(_game->getMod(), _game->getLanguage());
 		_game->getSavedGame()->setBattleGame(bgame);
 		bgame->setMissionType("STR_BASE_DEFENSE");
 		BattlescapeGenerator bgen = BattlescapeGenerator(_game);
 		bgen.setBase(_game->getSavedGame()->getSelectedBase());
-		bgen.setAlienCustomDeploy(_game->getMod()->getDeployment(temp_ufo->getCraftStats().missionCustomDeploy));
-		bgen.setAlienRace(temp_ufo->getAlienRace());
+		bgen.setAlienCustomDeploy(_game->getMod()->getDeployment(g_coopBaseDefense.missionCustomDeploy));
+		bgen.setAlienRace(g_coopBaseDefense.alienRace);
 		bgen.setWorldShade(shade);
 		Texture* globeTexture = _game->getMod()->getGlobe()->getTexture(texture);
 		bgen.setWorldTexture(globeTexture, globeTexture);
@@ -6006,7 +6028,16 @@ void GeoscapeState::handleBaseDefense(Base *base, Ufo *ufo)
 				_game->getCoopMod()->setGeoscapeState(this);
 				_game->getCoopMod()->_isMainCampaignBaseDefense = true;
 
-				temp_ufo = ufo;
+				// coop: snapshot the attacking UFO's parameters NOW. The coop base
+				// defense is started later in startCoopMission(), by when this Ufo
+				// has been freed - so capture the fields, never keep the pointer.
+				g_coopBaseDefense.pending = true;
+				g_coopBaseDefense.coop = ufo->getCoop();
+				g_coopBaseDefense.lon = ufo->getLongitude();
+				g_coopBaseDefense.lat = ufo->getLatitude();
+				g_coopBaseDefense.damagePercentage = ufo->getDamagePercentage();
+				g_coopBaseDefense.missionCustomDeploy = ufo->getCraftStats().missionCustomDeploy;
+				g_coopBaseDefense.alienRace = ufo->getAlienRace();
 
 				// PRD-J09 GAP-1: SHARED base defense. The garrison is the single shared
 				// world's roster, and the HOST (the only machine whose sim reaches this

tools/coop_test/test_coop_basedef_temp_ufo_uaf.py

@@ -0,0 +1,129 @@
+"""Regression test: coop base-defense must not crash on a dangling `temp_ufo`.
+
+Bug (reported as "save crashes on month end"): GeoscapeState::handleBaseDefense
+stashes the attacking retaliation UFO in the file-scope global `temp_ufo` and
+DEFERS the coop base-defense through a host<->client handshake. The UFO is freed
+during that window (retaliation cleanup / despawn), so GeoscapeState::
+startCoopMission dereferences a dangling pointer -> access violation. See
+GeoscapeState.cpp:169 / :6009 / :565 and .agents/repro/coop-basedef-uaf/.
+
+Scenario: host resumes the fixture coop save (a retaliation is inbound to a base),
+the registered client joins, then time advances until the base defense fires.
+
+  * PASS (exit 0): base-defense flow reaches the briefing/battle on both machines
+    with NO process crash -> the `temp_ufo` read is safe.
+  * FAIL (exit 2): a game process crashes (the UAF) -> bug present.
+  * FAIL (exit 3): the base defense never fired within the budget -> fixture/flow
+    regressed; the test proved nothing (do not treat as green).
+
+RED before the fix (crashes ~every run, verified), GREEN after.
+"""
+import glob
+import os
+import sys
+import time
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+FIXTURE = "coop_basedef_retaliation.sav"
+FIXTURE_PATH = os.path.join(HERE, "fixtures", FIXTURE)
+
+os.environ.setdefault("SDL_VIDEODRIVER", "dummy")
+os.environ.setdefault("SDL_AUDIODRIVER", "dummy")
+sys.path.insert(0, HERE)
+import harness  # noqa: E402
+import session  # noqa: E402
+import geo  # noqa: E402
+from harness import GameClient, make_user_dir  # noqa: E402
+
+PORT = "47934"
+RELEASE_DIR = os.path.dirname(harness.EXE)
+BATTLE_STATES = ("BriefingState", "BattlescapeState")
+
+
+def crash_logs():
+    hits = []
+    for root in (harness.TEST_ROOT, RELEASE_DIR):
+        hits += glob.glob(os.path.join(root, "**", "crash_*.log"), recursive=True)
+    return set(hits)
+
+
+def in_battle(gc):
+    """Interest: fires once the base-defense flow leaves the geoscape."""
+    return any(s in geo.top_state(gc) for s in BATTLE_STATES) or None
+
+
+def main():
+    assert os.path.isfile(harness.EXE), "no exe (set OXC_TEST_EXE): " + harness.EXE
+    assert os.path.isfile(FIXTURE_PATH), "missing fixture: " + FIXTURE_PATH
+    host_dir = make_user_dir("bduaf_host", saves=[FIXTURE_PATH])
+    client_dir = make_user_dir("bduaf_client")
+    pre = crash_logs()
+
+    host = GameClient("host", 48831, host_dir)
+    client = GameClient("client", 48832, client_dir)
+    host.spawn(); host.connect()
+    client.spawn(); client.connect()
+
+    battle_reached = False
+    note = None
+    try:
+        host.ok({"cmd": "load_save_menu", "file": FIXTURE})
+        host.wait_for("host window", lambda: session._has_state(host, "HostMenu"))
+        host.ok({"cmd": "host_tcp", "server": "TestSrv", "port": PORT, "player": "HostPlayer"})
+        host.wait_for("resume lobby", lambda: session._has_state(host, "LobbyMenu"))
+
+        client.ok({"cmd": "join_tcp", "ip": "127.0.0.1", "port": PORT, "player": "ClientPlayer"})
+        client.wait_for("client lobby", lambda: session._has_state(client, "LobbyMenu"))
+
+        host.wait_for("resume accepted",
+                      lambda: host.cmd({"cmd": "lobby_resume_campaign"}).get("ok") or None,
+                      timeout=60, interval=2.0)
+        host.wait_for("client world ack",
+                      lambda: host.cmd({"cmd": "get_coop"}).get("resumeAck") or None,
+                      timeout=120)
+        host.ok({"cmd": "coop_dialog_back"})
+        time.sleep(2.0)
+        geo.drain_popups(host); geo.drain_popups(client)
+
+        # Advance until the base defense fires (interest) or the budget elapses.
+        res = geo.skip_ingame_time(
+            host, client, minutes=12 * 24 * 60,
+            interest=lambda gc: in_battle(gc),
+            dismiss=True, real_timeout=180, stuck_timeout=None)
+        battle_reached = res.get("hit") is not None or in_battle(host) or in_battle(client)
+    except (ConnectionError, OSError) as e:
+        note = f"{type(e).__name__}: {e}"   # socket dropped mid-command == a crash
+    except Exception as e:
+        note = f"{type(e).__name__}: {e}"
+
+    time.sleep(1.5)
+    new_logs = sorted(crash_logs() - pre)
+    rc_host, rc_client = host.proc.poll(), client.proc.poll()
+    crashed = (rc_host is not None) or (rc_client is not None) or bool(new_logs)
+
+    print("==== RESULT ====")
+    print("battle_reached:", bool(battle_reached), "| crashed:", crashed,
+          "| host rc:", rc_host, "client rc:", rc_client, "| note:", note)
+    if new_logs:
+        print("crash logs:", [os.path.basename(x) for x in new_logs])
+        with open(new_logs[0], "r", errors="replace") as f:
+            sys.stdout.write(f.read())
+
+    for gc in (host, client):
+        try:
+            gc.shutdown()
+        except Exception:
+            pass
+
+    if crashed:
+        print("FAIL: coop base-defense crashed (temp_ufo use-after-free)")
+        sys.exit(2)
+    if not battle_reached:
+        print("INCONCLUSIVE: base defense never fired within budget")
+        sys.exit(3)
+    print("PASS: coop base-defense reached the battle with no crash")
+    sys.exit(0)
+
+
+if __name__ == "__main__":
+    main()