Skip to content

Fix connecting via Host Key#1

Open
fizzxed wants to merge 2 commits into
mmonad:merge-upstreamfrom
fizzxed:fix/ssh_hosts
Open

Fix connecting via Host Key#1
fizzxed wants to merge 2 commits into
mmonad:merge-upstreamfrom
fizzxed:fix/ssh_hosts

Conversation

@fizzxed

@fizzxed fizzxed commented Mar 17, 2026

Copy link
Copy Markdown

Previously if we passed a ssh host key, e.g. zmosh attach -r foo dev
where the ssh config file looked like

Host foo
    HostName actual.host.address

It would fail to connect due to trying to connect to foo after the ssh bootstrap

@ww7

ww7 commented Mar 27, 2026

Copy link
Copy Markdown

Backward compatibility issue with ZMX_CONNECT protocol change

This breaks compatibility when the client and server are running different versions. If an updated client connects to an older server (or vice versa), the parser fails with InvalidPort because it misinterprets
the fields.

Additionally, connectRemote returns result.host which is a slice into a stack-local buffer (buf[512]). Once the function returns, this becomes a dangling pointer — a use-after-free that causes the UDP connection
to target a garbage address and hang.

Suggested fix: Make parseConnectLine detect both formats by checking whether the 4th field parses as a port number. If it does, treat field 3 as host (new format); otherwise fall back to the old format (field 3 = port, field 4 = key). When the host comes from the connect line, alloc.dupe it to avoid the dangling pointer. When using the old format, fall back to the SSH host argument.

connectRemote returned a host slice pointing into a stack-local buffer.
Once the function returned, the slice became a dangling pointer. Dupe
the host onto the heap so it outlives the call.
@fizzxed

fizzxed commented Mar 27, 2026

Copy link
Copy Markdown
Author

Down to fix with the suggestion if backwards compatibility is an issue. Considering This isn't yet version 1.0 I didn't put a thought to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants