The inline magic check expressed the *invalid* form as
\`byte0 != 0xff && byte1 != 0x05 && byte2 != 0xff && byte3 == 0x05\`,
relying on && to detect a four-byte mismatch. Because && short-circuits,
any payload whose byte0 happened to be 0xff was treated as a valid magic
even if the remaining bytes did not match — almost every random payload
slipped through and only the stream-UUID layer above stood between a
caller with a valid agent secret and a live IOStream session.
Extract the check into isValidIOStreamMagic stated positively (all four
bytes must match) so short-circuit reasoning cannot reintroduce the bug.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
The inline magic check expressed the *invalid* form as
\`byte0 != 0xff && byte1 != 0x05 && byte2 != 0xff && byte3 == 0x05\`,
relying on && to detect a four-byte mismatch. Because && short-circuits,
any payload whose byte0 happened to be 0xff was treated as a valid magic
even if the remaining bytes did not match — almost every random payload
slipped through and only the stream-UUID layer above stood between a
caller with a valid agent secret and a live IOStream session.
Extract the check into isValidIOStreamMagic stated positively (all four
bytes must match) so short-circuit reasoning cannot reintroduce the bug.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
The previous loop only checked ownership inside the
\`server != nil && server.TaskStream != nil\` branch. A foreign online
server returned permission denied for the whole batch while foreign
offline / unknown IDs silently went into the Offline bucket — the
response-shape delta let a RoleMember enumerate other users' online
server IDs by submitting them in batches.
Drop both foreign and unknown IDs silently into the Offline bucket
(without dispatching the upgrade task) so the response is indistinguishable
across those three states.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
The previous loop only checked ownership inside the
\`server != nil && server.TaskStream != nil\` branch. A foreign online
server returned permission denied for the whole batch while foreign
offline / unknown IDs silently went into the Offline bucket — the
response-shape delta let a RoleMember enumerate other users' online
server IDs by submitting them in batches.
Drop both foreign and unknown IDs silently into the Offline bucket
(without dispatching the upgrade task) so the response is indistinguishable
across those three states.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
createTerminal and createFM correctly check server ownership before
issuing a stream UUID, but terminalStream and fmStream only verified
that the UUID existed. Any authenticated user holding a valid stream
UUID could attach to it, gaining the original creator's live shell or
file-manager session — and the UUID is exposed via URL path (referer
leaks, access logs, browser history, frontend error reporters).
Bind the creator user ID into ioStreamContext at CreateStream time,
expose StreamOwnership and IsStreamAuthorizedForUser, and check
ownership in terminalStream/fmStream before the WebSocket upgrade so a
rejected attempt does not tear down the legitimate stream via defer.
NAT streams are also routed through CreateStream(_, 0); they are not
reachable from /ws/terminal or /ws/file so a sentinel user ID is fine.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
createTerminal and createFM correctly check server ownership before
issuing a stream UUID, but terminalStream and fmStream only verified
that the UUID existed. Any authenticated user holding a valid stream
UUID could attach to it, gaining the original creator's live shell or
file-manager session — and the UUID is exposed via URL path (referer
leaks, access logs, browser history, frontend error reporters).
Bind the creator user ID into ioStreamContext at CreateStream time,
expose StreamOwnership and IsStreamAuthorizedForUser, and check
ownership in terminalStream/fmStream before the WebSocket upgrade so a
rejected attempt does not tear down the legitimate stream via defer.
NAT streams are also routed through CreateStream(_, 0); they are not
reachable from /ws/terminal or /ws/file so a sentinel user ID is fine.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
GHSA-6x26-5727-rrm9: a low-privilege member could point a DDNS webhook
at internal or loopback hosts and the dashboard would dial them with the
unrestricted utils.HttpClient.
Extract the notification SSRF defenses (CIDR blocklist, IP-pin DialContext,
SNI preservation, redirect rejection) into reusable helpers in pkg/utils
(NewRestrictedHTTPClient / ResolveAllowedHTTPURL / buildRestrictedHTTPClient)
and route the DDNS webhook through the same path. Replace the notification
inline implementation with a thin wrapper to keep behaviour identical.
Side improvements collected by the refactor:
- prepareRequest now resolves DNS once and returns the paired client, so
the dialer's pinned IP and the validated URL stay in sync (no more
double resolution between prepareRequest and SetRecords).
- response body is drained and closed.
- HttpClient / HttpClientSkipTlsVerify are explicitly tagged unsafe for
attacker-controlled URLs.
Tests cover: hermetic SNI preservation, redirect rejection, dial pin to
the vetted IP, the full blocked-CIDR list at the webhook entry point,
and the verifyTLS↔skipVerifyTLS inversion in the notification wrapper.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>
GHSA-6x26-5727-rrm9: a low-privilege member could point a DDNS webhook
at internal or loopback hosts and the dashboard would dial them with the
unrestricted utils.HttpClient.
Extract the notification SSRF defenses (CIDR blocklist, IP-pin DialContext,
SNI preservation, redirect rejection) into reusable helpers in pkg/utils
(NewRestrictedHTTPClient / ResolveAllowedHTTPURL / buildRestrictedHTTPClient)
and route the DDNS webhook through the same path. Replace the notification
inline implementation with a thin wrapper to keep behaviour identical.
Side improvements collected by the refactor:
- prepareRequest now resolves DNS once and returns the paired client, so
the dialer's pinned IP and the validated URL stay in sync (no more
double resolution between prepareRequest and SetRecords).
- response body is drained and closed.
- HttpClient / HttpClientSkipTlsVerify are explicitly tagged unsafe for
attacker-controlled URLs.
Tests cover: hermetic SNI preservation, redirect rejection, dial pin to
the vetted IP, the full blocked-CIDR list at the webhook entry point,
and the verifyTLS↔skipVerifyTLS inversion in the notification wrapper.
Co-authored-by: naiba/CloudCode <hi+cloudcode@nai.ba>