diff options
Diffstat (limited to 'internal/net')
| -rw-r--r-- | internal/net/conn.go | 14 | ||||
| -rw-r--r-- | internal/net/iac_test.go | 15 | ||||
| -rw-r--r-- | internal/net/server.go | 9 | ||||
| -rw-r--r-- | internal/net/web.go | 4 | ||||
| -rw-r--r-- | internal/net/web_test.go | 10 |
5 files changed, 42 insertions, 10 deletions
diff --git a/internal/net/conn.go b/internal/net/conn.go index d8c867e..df473f5 100644 --- a/internal/net/conn.go +++ b/internal/net/conn.go @@ -196,7 +196,19 @@ func stripIAC(s string) string { i += end + 4 continue } - i += 3 + if b[i+1] == iac { + // IAC IAC is an escaped data byte, not a command. + out = append(out, iac) + i += 2 + continue + } + // WILL/WONT/DO/DONT carry an option byte; every other IAC command is + // the two-byte form and must not eat the following input byte. + if b[i+1] == will || b[i+1] == wont || b[i+1] == do || b[i+1] == dont { + i += 3 + } else { + i += 2 + } if i > len(b) { i = len(b) } diff --git a/internal/net/iac_test.go b/internal/net/iac_test.go index 71570db..6f91f19 100644 --- a/internal/net/iac_test.go +++ b/internal/net/iac_test.go @@ -15,3 +15,18 @@ func TestStripIACDropsTruncatedSequence(t *testing.T) { t.Fatalf("stripIAC(subnegotiation) = %q, want xy", got) } } + +func TestStripIACPreservesInputAfterTwoByteCommand(t *testing.T) { + // A 2-byte command (NOP) in the middle must not eat the following byte. + if got := stripIAC("a" + string([]byte{iac, 0xf1}) + "b"); got != "ab" { + t.Fatalf("stripIAC(mid two-byte command) = %q, want %q", got, "ab") + } + // A 3-byte negotiation command consumes its option byte. + if got := stripIAC("a" + string([]byte{iac, will, echo}) + "b"); got != "ab" { + t.Fatalf("stripIAC(three-byte command) = %q, want %q", got, "ab") + } + // IAC IAC is an escaped literal 0xFF, not a command. + if got := stripIAC("a" + string([]byte{iac, iac}) + "b"); got != "a"+string([]byte{iac})+"b" { + t.Fatalf("stripIAC(escaped IAC) = %q, want literal 0xFF", got) + } +} diff --git a/internal/net/server.go b/internal/net/server.go index 0854426..d1a0492 100644 --- a/internal/net/server.go +++ b/internal/net/server.go @@ -570,7 +570,14 @@ func (h *Hub) RekeyRooms(idMap map[int]int) { moved := make(map[int]map[*Session]bool, len(h.rooms)) for id, set := range h.rooms { if newID, ok := idMap[id]; ok && newID != id { - moved[newID] = set + if moved[newID] == nil { + moved[newID] = make(map[*Session]bool) + } + // Merge rather than assign: a many-to-one idMap (A->C, B->C) + // would otherwise clobber the first source bucket's sessions. + for s := range set { + moved[newID][s] = true + } delete(h.rooms, id) } } diff --git a/internal/net/web.go b/internal/net/web.go index 0ba0c98..d21fb0c 100644 --- a/internal/net/web.go +++ b/internal/net/web.go @@ -34,10 +34,6 @@ var upgrader = websocket.Upgrader{ strings.EqualFold(origin, "https://"+hostname) { return true } - if hn, _, err := net.SplitHostPort(r.RemoteAddr); err == nil && - (hn == "127.0.0.1" || hn == "::1") { - return true - } log.Printf("ws rejected origin: %s (host: %s)", origin, r.Host) return false }, diff --git a/internal/net/web_test.go b/internal/net/web_test.go index d16b31a..20bf7c8 100644 --- a/internal/net/web_test.go +++ b/internal/net/web_test.go @@ -10,8 +10,9 @@ import ( ) // The browser transport previously had no tests. CheckOrigin is the security -// boundary for the /ws upgrade; test it directly with a non-loopback peer so -// the loopback bypass does not mask a hostile Origin. +// boundary for the /ws upgrade; test it directly, including a loopback peer: +// being on the server host must not re-open the cross-site WebSocket hole for +// a hostile browser. func TestWebSocketCheckOrigin(t *testing.T) { cases := []struct { name string @@ -25,7 +26,7 @@ func TestWebSocketCheckOrigin(t *testing.T) { {"same host https", "https://mud.example:4000", "mud.example:4000", "203.0.113.5:1234", true}, {"same hostname no port", "http://mud.example", "mud.example:4000", "203.0.113.5:1234", true}, {"hostile origin", "http://evil.example", "mud.example:4000", "203.0.113.5:1234", false}, - {"loopback bypass", "http://evil.example", "mud.example:4000", "127.0.0.1:1234", true}, + {"loopback hostile origin", "http://evil.example", "mud.example:4000", "127.0.0.1:1234", false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -41,7 +42,8 @@ func TestWebSocketCheckOrigin(t *testing.T) { } // The /ws upgrade wires a live session and writer. Drive it end-to-end over a -// real HTTP test server (loopback, so the origin check passes). +// real HTTP test server (the dialer sends no Origin header, which CheckOrigin +// accepts). func TestWebSocketUpgradeAccepted(t *testing.T) { s := &Server{hub: NewHub()} done := make(chan struct{}) |
