Skip to content

Commit 7b84c4d

Browse files
committed
Pass a message size limit to the stream command decoder
Bump github.com/centrifugal/protocol to v0.21.0, which now requires a positive message size limit when constructing a stream command decoder (GetStreamCommandDecoder, the unbounded shortcut, was removed). An unbounded decoder over untrusted input can be driven to allocate arbitrary memory by a single frame declaring a huge length. See GHSA-4r3x-2rwr-6w65. HandleReadFrame now takes a messageSizeLimit and passes it to GetStreamCommandDecoderLimited, so an oversized declared length is rejected before it is allocated. Each transport supplies its own limit, coerced to a positive default: - WebSocket: MessageSizeLimit, or DecompressedMessageSizeLimit when compression is enabled (the decoder sees decoded bytes). - SSE / HTTP-stream: MaxRequestBodySize. - Emulation: the length of the already-buffered command bytes, which a single command cannot exceed. All coercions use <= 0 so a negative config cannot reach the decoder.
1 parent ccb335a commit 7b84c4d

7 files changed

Lines changed: 62 additions & 44 deletions

File tree

client_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func TestClientV2PingPong(t *testing.T) {
263263
for msg := range messages {
264264
if string(msg) == "{}" {
265265
// PING
266-
HandleReadFrame(client, bytes.NewReader([]byte("{}")))
266+
HandleReadFrame(client, bytes.NewReader([]byte("{}")), 1<<20)
267267
}
268268
}
269269
}()
@@ -2834,14 +2834,14 @@ func TestClientHandleEmptyData(t *testing.T) {
28342834
defer func() { _ = node.Shutdown(context.Background()) }()
28352835

28362836
client := newTestClient(t, node, "42")
2837-
proceed := HandleReadFrame(client, bytes.NewReader([]byte(nil)))
2837+
proceed := HandleReadFrame(client, bytes.NewReader([]byte(nil)), 1<<20)
28382838
require.False(t, proceed)
28392839
select {
28402840
case <-client.Context().Done():
28412841
case <-time.After(time.Second):
28422842
require.Fail(t, "client not closed")
28432843
}
2844-
proceed = HandleReadFrame(client, bytes.NewReader([]byte("test")))
2844+
proceed = HandleReadFrame(client, bytes.NewReader([]byte("test")), 1<<20)
28452845
require.False(t, proceed)
28462846
disconnect, proceed := client.dispatchCommand(&protocol.Command{}, 0)
28472847
require.Nil(t, disconnect)
@@ -2854,7 +2854,7 @@ func TestClientHandleBrokenData(t *testing.T) {
28542854
defer func() { _ = node.Shutdown(context.Background()) }()
28552855

28562856
client := newTestClient(t, node, "42")
2857-
proceed := HandleReadFrame(client, bytes.NewReader([]byte(`nd3487yt734y38&**&**`)))
2857+
proceed := HandleReadFrame(client, bytes.NewReader([]byte(`nd3487yt734y38&**&**`)), 1<<20)
28582858
require.False(t, proceed)
28592859
select {
28602860
case <-client.Context().Done():
@@ -2874,7 +2874,7 @@ func TestClientHandleCommandNotAuthenticated(t *testing.T) {
28742874
}}
28752875
data, err := json.Marshal(cmd)
28762876
require.NoError(t, err)
2877-
proceed := HandleReadFrame(client, bytes.NewReader(data))
2877+
proceed := HandleReadFrame(client, bytes.NewReader(data), 1<<20)
28782878
require.False(t, proceed)
28792879
select {
28802880
case <-client.Context().Done():
@@ -2932,7 +2932,7 @@ func TestClientHandleCommandWithoutID(t *testing.T) {
29322932
cmd := &protocol.Command{}
29332933
data, err := json.Marshal(cmd)
29342934
require.NoError(t, err)
2935-
proceed := HandleReadFrame(client, bytes.NewReader(data))
2935+
proceed := HandleReadFrame(client, bytes.NewReader(data), 1<<20)
29362936
require.False(t, proceed)
29372937
select {
29382938
case <-client.Context().Done():
@@ -2968,7 +2968,7 @@ func TestClientAlreadyAuthenticated(t *testing.T) {
29682968
cmd := &protocol.Command{Id: 2, Connect: &protocol.ConnectRequest{}}
29692969
data, err := json.Marshal(cmd)
29702970
require.NoError(t, err)
2971-
proceed := HandleReadFrame(client, bytes.NewReader(data))
2971+
proceed := HandleReadFrame(client, bytes.NewReader(data), 1<<20)
29722972
require.False(t, proceed)
29732973
select {
29742974
case <-client.Context().Done():
@@ -4899,7 +4899,7 @@ func BenchmarkClientRPC(b *testing.B) {
48994899

49004900
b.ResetTimer()
49014901
for i := 0; i < b.N; i++ {
4902-
decoder := protocol.GetStreamCommandDecoder(protocol.TypeProtobuf, bytes.NewReader(frame))
4902+
decoder := protocol.GetStreamCommandDecoderLimited(protocol.TypeProtobuf, bytes.NewReader(frame), 1<<20)
49034903
cmd, cmdProtocolSize, err := decoder.Decode()
49044904
require.NoError(b, err)
49054905
client.HandleCommand(cmd, cmdProtocolSize)

emulation.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ func (h *emulationSurveyHandler) HandleEmulation(e SurveyEvent, cb SurveyCallbac
165165
}
166166
go func() {
167167
reader := readerpool.GetBytesReader(data)
168-
_ = HandleReadFrame(client, reader)
168+
// data holds the already-buffered command bytes, so a single command
169+
// cannot exceed its length; +1 keeps the limit positive (the stream
170+
// decoder rejects a non-positive limit) and admits a command of exactly
171+
// len(data) bytes. The ingress body size was already bounded upstream.
172+
_ = HandleReadFrame(client, reader, int64(len(data))+1)
169173
readerpool.PutBytesReader(reader)
170174
cb(SurveyReply{})
171175
}()

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.25.0
44

55
require (
66
github.com/FZambia/eagle v0.2.0
7-
github.com/centrifugal/protocol v0.20.1-0.20260811164823-815457ed4d09
7+
github.com/centrifugal/protocol v0.21.0
88
github.com/cespare/xxhash/v2 v2.3.0
99
github.com/google/cel-go v0.30.0
1010
github.com/google/uuid v1.6.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYW
66
github.com/antlr4-go/antlr/v4 v4.13.1/go.mod h1:GKmUxMtwp6ZgGwZSva4eWPC5mS6vUAmOABFgjdkM7Nw=
77
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
88
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
9-
github.com/centrifugal/protocol v0.20.1-0.20260811164823-815457ed4d09 h1:8SJBvmcS/KXNu8h/79VyYgey4WNreVM9NR3NrGNLFSQ=
10-
github.com/centrifugal/protocol v0.20.1-0.20260811164823-815457ed4d09/go.mod h1:3pinAfcb+bH8Yp8Ewhf9QTRTzsQ9veP/QOrCk5D5SSE=
9+
github.com/centrifugal/protocol v0.21.0 h1:yagnxWBH7vK2+0+A0bmPAIOCaFqTvaLVWubkklS7Xyo=
10+
github.com/centrifugal/protocol v0.21.0/go.mod h1:3pinAfcb+bH8Yp8Ewhf9QTRTzsQ9veP/QOrCk5D5SSE=
1111
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
1212
github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
1313
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=

handler_http_stream.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,18 +62,21 @@ func (h *HTTPStreamHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
6262
protocolType = ProtocolTypeProtobuf
6363
}
6464

65+
// messageSizeLimit bounds both the request body and a single decoded command;
66+
// it must be positive since the stream decoder rejects a non-positive limit.
67+
messageSizeLimit := h.config.MaxRequestBodySize
68+
if messageSizeLimit <= 0 {
69+
messageSizeLimit = defaultMaxHTTPStreamingBodySize
70+
}
71+
6572
var requestData []byte
6673
if r.Method == http.MethodPost {
67-
maxBytesSize := h.config.MaxRequestBodySize
68-
if maxBytesSize == 0 {
69-
maxBytesSize = defaultMaxHTTPStreamingBodySize
70-
}
71-
r.Body = http.MaxBytesReader(w, r.Body, int64(maxBytesSize))
74+
r.Body = http.MaxBytesReader(w, r.Body, int64(messageSizeLimit))
7275
var err error
7376
requestData, err = io.ReadAll(r.Body)
7477
if err != nil {
7578
h.node.logger.log(newLogEntry(LogLevelInfo, "error reading http stream request body", map[string]any{"error": err.Error()}))
76-
if len(requestData) >= maxBytesSize {
79+
if len(requestData) >= messageSizeLimit {
7780
w.WriteHeader(http.StatusRequestEntityTooLarge)
7881
return
7982
}
@@ -120,7 +123,7 @@ func (h *HTTPStreamHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
120123
rc := http.NewResponseController(w)
121124

122125
reader := readerpool.GetBytesReader(requestData)
123-
_ = HandleReadFrame(c, reader)
126+
_ = HandleReadFrame(c, reader, int64(messageSizeLimit))
124127
readerpool.PutBytesReader(reader)
125128

126129
sendAck := func() {

handler_sse.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
5050
return
5151
}
5252

53+
// messageSizeLimit bounds both the request body and a single decoded command;
54+
// it must be positive since the stream decoder rejects a non-positive limit.
55+
messageSizeLimit := h.config.MaxRequestBodySize
56+
if messageSizeLimit <= 0 {
57+
messageSizeLimit = defaultMaxSSEBodySize
58+
}
59+
5360
var requestData []byte
5461
if r.Method == http.MethodGet {
5562
requestDataString := r.URL.Query().Get(connectUrlParam)
@@ -61,16 +68,12 @@ func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
6168
return
6269
}
6370
} else if r.Method == http.MethodPost {
64-
maxBytesSize := h.config.MaxRequestBodySize
65-
if maxBytesSize == 0 {
66-
maxBytesSize = defaultMaxSSEBodySize
67-
}
68-
r.Body = http.MaxBytesReader(w, r.Body, int64(maxBytesSize))
71+
r.Body = http.MaxBytesReader(w, r.Body, int64(messageSizeLimit))
6972
var err error
7073
requestData, err = io.ReadAll(r.Body)
7174
if err != nil {
7275
h.node.logger.log(newLogEntry(LogLevelInfo, "error reading sse request body", map[string]any{"error": err.Error()}))
73-
if len(requestData) >= maxBytesSize {
76+
if len(requestData) >= messageSizeLimit {
7477
w.WriteHeader(http.StatusRequestEntityTooLarge)
7578
return
7679
}
@@ -128,7 +131,7 @@ func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
128131
_ = rc.SetWriteDeadline(time.Time{})
129132

130133
reader := readerpool.GetBytesReader(requestData)
131-
_ = HandleReadFrame(c, reader)
134+
_ = HandleReadFrame(c, reader, int64(messageSizeLimit))
132135
readerpool.PutBytesReader(reader)
133136

134137
sendAck := func() {

handler_websocket.go

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -223,20 +223,22 @@ func (s *WebsocketHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
223223
writeTimeout = 1 * time.Second
224224
}
225225
messageSizeLimit := s.config.MessageSizeLimit
226-
if messageSizeLimit == 0 {
226+
if messageSizeLimit <= 0 {
227227
messageSizeLimit = 65536 // 64KB
228228
}
229-
if messageSizeLimit > 0 {
230-
conn.SetReadLimit(int64(messageSizeLimit))
231-
}
229+
conn.SetReadLimit(int64(messageSizeLimit))
230+
// The stream command decoder parses decoded command bytes, so it is bounded
231+
// by the decompressed message size when compression is enabled, otherwise by
232+
// the wire message size. It must be positive - the decoder rejects a
233+
// non-positive limit.
234+
decoderMessageSizeLimit := int64(messageSizeLimit)
232235
if compression {
233236
decompressedMessageSizeLimit := s.config.DecompressedMessageSizeLimit
234-
if decompressedMessageSizeLimit == 0 {
237+
if decompressedMessageSizeLimit <= 0 {
235238
decompressedMessageSizeLimit = messageSizeLimit * defaultWebsocketDecompressedMessageSizeLimitMultiplier
236239
}
237-
if decompressedMessageSizeLimit > 0 {
238-
conn.SetDecompressedReadLimit(int64(decompressedMessageSizeLimit))
239-
}
240+
conn.SetDecompressedReadLimit(int64(decompressedMessageSizeLimit))
241+
decoderMessageSizeLimit = int64(decompressedMessageSizeLimit)
240242
}
241243

242244
if useFramePingPong {
@@ -309,7 +311,7 @@ func (s *WebsocketHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
309311
// costs a goroutine but no allocation. See ProcessCommandsOffReadLoop.
310312
var handoff *frameHandoff
311313
if s.config.ProcessCommandsOffReadLoop {
312-
handoff = newFrameHandoff(c)
314+
handoff = newFrameHandoff(c, decoderMessageSizeLimit)
313315
}
314316

315317
for {
@@ -324,7 +326,7 @@ func (s *WebsocketHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
324326
if handoff != nil {
325327
proceed = handoff.handle(r)
326328
} else {
327-
proceed = HandleReadFrame(c, r)
329+
proceed = HandleReadFrame(c, r, decoderMessageSizeLimit)
328330
}
329331
if !proceed {
330332
break
@@ -365,17 +367,18 @@ func (s *WebsocketHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
365367
// Only the read loop writes r, and it does so before starting run and does not
366368
// touch it again until run has sent on done, so the two goroutines never race.
367369
type frameHandoff struct {
368-
c *Client
369-
r io.Reader
370-
done chan bool
370+
c *Client
371+
r io.Reader
372+
done chan bool
373+
messageSizeLimit int64
371374
}
372375

373-
func newFrameHandoff(c *Client) *frameHandoff {
374-
return &frameHandoff{c: c, done: make(chan bool, 1)}
376+
func newFrameHandoff(c *Client, messageSizeLimit int64) *frameHandoff {
377+
return &frameHandoff{c: c, done: make(chan bool, 1), messageSizeLimit: messageSizeLimit}
375378
}
376379

377380
func (h *frameHandoff) run() {
378-
h.done <- HandleReadFrame(h.c, h.r)
381+
h.done <- HandleReadFrame(h.c, h.r, h.messageSizeLimit)
379382
}
380383

381384
// handle processes one frame off the read loop and reports whether the
@@ -393,9 +396,14 @@ func (h *frameHandoff) handle(r io.Reader) bool {
393396
// HandleReadFrame is a helper to read Centrifuge commands from frame-based io.Reader and
394397
// process them. Frame-based means that EOF treated as the end of the frame, not the entire
395398
// connection close.
396-
func HandleReadFrame(c *Client, r io.Reader) bool {
399+
//
400+
// messageSizeLimit bounds the size of a single command and must be positive: the
401+
// underlying stream decoder rejects a non-positive limit, since the length prefix
402+
// is attacker-controlled and used as an allocation size. Callers pass the
403+
// transport's configured limit (coerced to a positive default).
404+
func HandleReadFrame(c *Client, r io.Reader, messageSizeLimit int64) bool {
397405
protoType := c.Transport().Protocol().toProto()
398-
decoder := protocol.GetStreamCommandDecoder(protoType, r)
406+
decoder := protocol.GetStreamCommandDecoderLimited(protoType, r, messageSizeLimit)
399407
defer protocol.PutStreamCommandDecoder(protoType, decoder)
400408

401409
hadCommands := false

0 commit comments

Comments
 (0)