diff --git a/internal/rcon/rcon.go b/internal/rcon/rcon.go index e7490ef..240d11b 100644 --- a/internal/rcon/rcon.go +++ b/internal/rcon/rcon.go @@ -211,11 +211,13 @@ func (c *Conn) nextID() int32 { // writePacket encodes one RCON packet: little-endian length, id, type, the // null-terminated body, and a trailing null byte. func writePacket(w io.Writer, id, typ int32, body string) error { + // Bounded on the body, before the int32 conversion: one past 2 GiB would + // wrap the length negative and slip under a check made after it. + if len(body) > maxPacketLen-minPacketLen { + return fmt.Errorf("rcon: outgoing packet too large: %d bytes", len(body)+minPacketLen) + } bodyBytes := []byte(body) length := int32(4 + 4 + len(bodyBytes) + 2) - if length > maxPacketLen { - return fmt.Errorf("rcon: outgoing packet too large: %d bytes", length) - } buf := make([]byte, 0, 4+length) buf = appendInt32(buf, length) buf = appendInt32(buf, id) diff --git a/internal/rcon/rcon_test.go b/internal/rcon/rcon_test.go index ca4954a..e548580 100644 --- a/internal/rcon/rcon_test.go +++ b/internal/rcon/rcon_test.go @@ -169,6 +169,27 @@ func TestDialAndExecute(t *testing.T) { } } +// A command goes out in one packet of at most 4096 bytes; a longer one is +// refused before anything is sent. +func TestExecuteRefusesAnOversizeCommand(t *testing.T) { + longest := strings.Repeat("a", 4096-10) // 10: id, type, two terminators + f := startFakeRCON(t, "s3cret", map[string]string{longest: "ok"}) + defer f.stop() + + c, err := rcon.Dial(f.addr(), "s3cret", 2*time.Second) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer c.Close() + + if got, err := c.Execute(longest); err != nil || got != "ok" { + t.Fatalf("Execute(%d bytes) = %q, %v; want ok", len(longest), got, err) + } + if _, err := c.Execute(longest + "a"); err == nil || !strings.Contains(err.Error(), "too large") { + t.Fatalf("Execute(%d bytes) err = %v, want the too-large refusal", len(longest)+1, err) + } +} + func TestDialAuthFailure(t *testing.T) { f := startFakeRCON(t, "correct-horse", nil) defer f.stop()