Fix: Universal access port TOCTOU race and disconnect issues (#22554)

* Please describe the intent of your changes in a clear fashion.
This PR addresses two related bugs concerning the universal access port
and cable:

**1. TOCTOU Race Condition Leading to Crash (SERVER-PROD-X9)**
* **Root Cause:** When inserting a universal access cable into an IPC's
access port, a check for `access_port.internal_port` occupancy occurs
before a `do_mob` delay (which can include a consent dialog and a
2-second wait). If another action or player inserts something into the
port during this delay, the subsequent call to `insert_item()` would
find `internal_port` already occupied and trigger a `crash_with()`,
leading to a server crash.
*   **Fix:**
* A re-check for `access_port.internal_port` occupancy and the continued
existence of the `access_port` itself has been added immediately after
the `do_mob` delay in `access_cable/attack()`. If the port is now
occupied or no longer exists, the insertion attempt is gracefully
aborted with a user message.
* The `crash_with()` call in `insert_item()` has been replaced with a
`log_debug()` and an early return, ensuring that any future unexpected
state in `internal_port` leads to graceful degradation rather than a
server crash.

**2. Stale `internal_port` on Cable Disconnect**
* **Root Cause:** The `access_port` did not override the generic
`remove_cable()` proc. When a cable was retracted or removed,
`access_port.internal_port` was not explicitly cleared, causing the port
to appear permanently occupied even after the cable was gone.
*   **Fix:**
* An override for `remove_cable()` has been added to
`/obj/item/organ/internal/machine/access_port`. This ensures that
`clear_port()` is called, properly nulling `internal_port` when the
cable is removed.

* Please make sure that, in the case of mapping changes, you include
images of these changes in the PR's description.
* Please make sure to mark your PR as wip or review required by making a
comment with !wip or !review required
* If you include sprites/sounds/... (assets) that you have not created
yourself specify the license and original author below.
* Ensure that you also credit them in the appropriate location /
changelog as specified in the contributor guidelines

### Asset Licenses
The following assets that **have not** been created by myself are
included in this PR:

| Path | Original Author | License |
| --- | --- | --- |
| icons/example.dmi | ExamplePerson (Example Station) | CC0 |

Fixes SERVER-PROD-X9

---------

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
Co-authored-by: VMSolidus <evilexecutive@gmail.com>
This commit is contained in:
sentry[bot]
2026-06-01 10:33:15 +00:00
committed by GitHub
co-authored by sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> VMSolidus
parent d82b463aca
commit 643c692784
2 changed files with 23 additions and 1 deletions
@@ -77,11 +77,13 @@
SIGNAL_HANDLER
if(internal_port)
crash_with("Insert_item with [jack] on access port called with [internal_port] of [owner] already present!")
return FALSE
internal_port = jack
jack.forceMove(src)
RegisterSignal(internal_port, COMSIG_QDELETING, PROC_REF(clear_port))
to_chat(owner, SPAN_MACHINE_WARNING("Internal firewall notice: [internal_port] inserted into [src]."))
return TRUE
/**
* This proc is called whenever the access cable is, for some reason, qdeleted (like with an explosion).
@@ -111,9 +113,16 @@
/obj/item/organ/internal/machine/access_port/insert_cable(obj/item/access_cable/cable, mob/user)
. = ..()
insert_item(cable)
if (!insert_item(cable))
return
cable.create_cable(owner)
/obj/item/organ/internal/machine/access_port/remove_cable(obj/item/access_cable/cable)
. = ..()
if(internal_port == cable)
clear_port()
/obj/item/organ/internal/machine/access_port/cable_interact(obj/item/access_cable/cable, mob/user)
var/obj/item/organ/internal/machine/internal_diagnostics/diagnostics_unit = owner.internal_organs_by_name[BP_DIAGNOSTICS_SUITE]
if(!diagnostics_unit)
@@ -277,6 +286,15 @@
user.visible_message(SPAN_WARNING("[user] tries to jack \the [src] into [human]'s access port..."))
if(!do_mob(user, human, 2 SECONDS))
return
// Re-check after the delay: port state may have changed during consent/wait window
var/obj/item/organ/internal/machine/access_port/fresh_port = human.internal_organs_by_name[BP_ACCESS_PORT]
if(!fresh_port)
to_chat(user, SPAN_WARNING("[human] no longer has an access port!"))
return
if(fresh_port.internal_port)
to_chat(user, SPAN_WARNING("The access port is now occupied by [fresh_port.internal_port]!"))
return
access_port = fresh_port
user.visible_message(SPAN_WARNING("[user] jacks \the [src] into [human]'s access port!"))
else