Skip to content

Commit cdc039d

Browse files
Tweshomarroth
authored andcommitted
Reject a hostile Content-Length instead of panicking
A receiver answering with "Content-Length: -1" crashes the sender. The value is parsed with Sscanf into an int, is not checked, and reaches make([]byte, contentLength) in readPlaintextHTTPResponse, which panics with "makeslice: len out of range". Nothing recovers it. Reproduced end to end over a net.Pipe standing in for the receiver: the panic happens inside readPlaintextHTTPResponse, not in a helper. Also bounds the positive case. Content-Length is attacker- or bug-controlled and was used unbounded, so "Content-Length: 2147483647" asks the sender for a 2 GB allocation before a single body byte arrives. Control-channel bodies here are small plists; the 8 MB limit is far above anything legitimate. The other four make([]byte, n) sites in this package are fine and are left alone -- readEncryptedFrame already bounds its uint16 length, hkdfSHA512 and padTo take caller-supplied sizes, and audio.go's n comes from a Read. Tests cover negative, large-negative and oversized headers, plus a well-formed response to show the normal path still works. Removing the guard makes the first two fail with the original panic.
1 parent 8ccea5f commit cdc039d

2 files changed

Lines changed: 120 additions & 0 deletions

File tree

internal/airplay/client.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,9 @@ func (c *AirPlayClient) readPlaintextHTTPResponse() ([]byte, map[string]string,
352352
dbg("[READ] plaintext response header:\n%s", header)
353353
statusCode, contentLength, headers := parseHTTPHeader(header)
354354
dbg("[READ] status=%d content-length=%d", statusCode, contentLength)
355+
if err := validateContentLength(contentLength); err != nil {
356+
return nil, headers, err
357+
}
355358

356359
if statusCode < 200 || statusCode >= 300 {
357360
// Drain body if present
@@ -696,3 +699,23 @@ func (mc *mirrorCipher) EncryptFrame(payload []byte) []byte {
696699

697700
return out
698701
}
702+
703+
// maxResponseBody bounds what a receiver can make the sender allocate from a
704+
// Content-Length header. Control-channel bodies here are small plists; this is
705+
// far above anything legitimate and far below anything that would exhaust
706+
// memory.
707+
const maxResponseBody = 8 << 20
708+
709+
// validateContentLength rejects a Content-Length the sender cannot safely act
710+
// on. A negative value is the important one: it reaches make([]byte, n) and
711+
// panics with "makeslice: len out of range", so a receiver answering
712+
// "Content-Length: -1" crashes the sender outright.
713+
func validateContentLength(n int) error {
714+
if n < 0 {
715+
return fmt.Errorf("invalid negative Content-Length %d", n)
716+
}
717+
if n > maxResponseBody {
718+
return fmt.Errorf("Content-Length %d exceeds the %d byte limit", n, maxResponseBody)
719+
}
720+
return nil
721+
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package airplay
2+
3+
import (
4+
"net"
5+
"strings"
6+
"testing"
7+
"time"
8+
)
9+
10+
// A receiver answering with a negative Content-Length used to crash the sender:
11+
// the value reached make([]byte, n) and panicked with "makeslice: len out of
12+
// range". It is now rejected as a parse error.
13+
func TestReadResponseRejectsHostileContentLength(t *testing.T) {
14+
for _, tc := range []struct {
15+
name string
16+
header string
17+
want string
18+
}{
19+
{"negative", "Content-Length: -1", "negative"},
20+
{"large negative", "Content-Length: -2147483648", "negative"},
21+
{"absurdly large", "Content-Length: 2147483647", "exceeds"},
22+
} {
23+
t.Run(tc.name, func(t *testing.T) {
24+
client, server := net.Pipe()
25+
defer client.Close()
26+
defer server.Close()
27+
28+
go func() {
29+
server.Write([]byte("RTSP/1.0 200 OK\r\n" + tc.header + "\r\n\r\n"))
30+
time.Sleep(time.Second)
31+
}()
32+
33+
c := &AirPlayClient{conn: client}
34+
done := make(chan error, 1)
35+
go func() {
36+
defer func() {
37+
if r := recover(); r != nil {
38+
t.Errorf("panicked instead of returning an error: %v", r)
39+
done <- nil
40+
}
41+
}()
42+
_, _, err := c.readPlaintextHTTPResponse()
43+
done <- err
44+
}()
45+
46+
select {
47+
case err := <-done:
48+
if err == nil {
49+
t.Fatal("expected an error")
50+
}
51+
if !strings.Contains(err.Error(), tc.want) {
52+
t.Fatalf("error %q does not mention %q", err, tc.want)
53+
}
54+
case <-time.After(5 * time.Second):
55+
t.Fatal("timed out")
56+
}
57+
})
58+
}
59+
}
60+
61+
// A well-formed response must still be read normally.
62+
func TestReadResponseAcceptsValidContentLength(t *testing.T) {
63+
client, server := net.Pipe()
64+
defer client.Close()
65+
defer server.Close()
66+
67+
go func() {
68+
server.Write([]byte("RTSP/1.0 200 OK\r\nContent-Length: 5\r\n\r\nhello"))
69+
time.Sleep(time.Second)
70+
}()
71+
72+
c := &AirPlayClient{conn: client}
73+
body, headers, err := c.readPlaintextHTTPResponse()
74+
if err != nil {
75+
t.Fatalf("unexpected error: %v", err)
76+
}
77+
if string(body) != "hello" {
78+
t.Fatalf("body = %q, want %q", body, "hello")
79+
}
80+
if headers["content-length"] != "5" {
81+
t.Fatalf("content-length header = %q", headers["content-length"])
82+
}
83+
}
84+
85+
func TestValidateContentLength(t *testing.T) {
86+
for _, tc := range []struct {
87+
n int
88+
ok bool
89+
}{
90+
{-1, false}, {0, true}, {1, true},
91+
{maxResponseBody, true}, {maxResponseBody + 1, false},
92+
} {
93+
if err := validateContentLength(tc.n); (err == nil) != tc.ok {
94+
t.Errorf("validateContentLength(%d): err=%v, want ok=%v", tc.n, err, tc.ok)
95+
}
96+
}
97+
}

0 commit comments

Comments
 (0)