Skip to content

Commit 9ed1a63

Browse files
committed
limit webhook payload size in ValidatePayloadFromBody
1 parent a276aa8 commit 9ed1a63

2 files changed

Lines changed: 59 additions & 3 deletions

File tree

github/messages.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ const (
4242
EventTypeHeader = "X-Github-Event"
4343
// DeliveryIDHeader is the GitHub header key used to pass the unique ID for the webhook event.
4444
DeliveryIDHeader = "X-Github-Delivery"
45+
46+
// maxPayloadSize is the maximum size of a GitHub webhook payload.
47+
// GitHub documents a 25 MB limit for webhook payloads.
48+
maxPayloadSize = 25 * 1024 * 1024
4549
)
4650

4751
var (
@@ -146,7 +150,19 @@ func checkMAC(message, messageMAC, key []byte, hashFunc func() hash.Hash) bool {
146150
return hmac.Equal(messageMAC, expectedMAC)
147151
}
148152

149-
// messageMAC returns the hex-decoded HMAC tag from the signature and its
153+
// readPayloadBody reads the body from readable, enforcing maxPayloadSize.
154+
func readPayloadBody(readable io.Reader) ([]byte, error) {
155+
body, err := io.ReadAll(io.LimitReader(readable, maxPayloadSize+1))
156+
if err != nil {
157+
return nil, err
158+
}
159+
if len(body) > maxPayloadSize {
160+
return nil, errors.New("webhook payload exceeds maximum allowed size")
161+
}
162+
return body, nil
163+
}
164+
165+
150166
// corresponding hash function.
151167
func messageMAC(signature string) ([]byte, func() hash.Hash, error) {
152168
if signature == "" {
@@ -199,7 +215,7 @@ func ValidatePayloadFromBody(contentType string, readable io.Reader, signature s
199215
switch contentType {
200216
case "application/json":
201217
var err error
202-
if body, err = io.ReadAll(readable); err != nil {
218+
if body, err = readPayloadBody(readable); err != nil {
203219
return nil, err
204220
}
205221

@@ -213,7 +229,7 @@ func ValidatePayloadFromBody(contentType string, readable io.Reader, signature s
213229
const payloadFormParam = "payload"
214230

215231
var err error
216-
if body, err = io.ReadAll(readable); err != nil {
232+
if body, err = readPayloadBody(readable); err != nil {
217233
return nil, err
218234
}
219235

github/messages_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"encoding/json"
1111
"errors"
1212
"fmt"
13+
"io"
1314
"net/http"
1415
"net/url"
1516
"strings"
@@ -205,6 +206,16 @@ func (b *badReader) Read([]byte) (int, error) {
205206

206207
func (b *badReader) Close() error { return errors.New("bad reader") }
207208

209+
// infiniteReader is an io.Reader that returns zeros indefinitely.
210+
type infiniteReader struct{}
211+
212+
func (infiniteReader) Read(p []byte) (int, error) {
213+
for i := range p {
214+
p[i] = 0
215+
}
216+
return len(p), nil
217+
}
218+
208219
func TestValidatePayload_BadRequestBody(t *testing.T) {
209220
t.Parallel()
210221
tests := []struct {
@@ -228,6 +239,35 @@ func TestValidatePayload_BadRequestBody(t *testing.T) {
228239
}
229240
}
230241

242+
func TestValidatePayload_OversizedBody(t *testing.T) {
243+
t.Parallel()
244+
tests := []struct {
245+
contentType string
246+
}{
247+
{contentType: "application/json"},
248+
{contentType: "application/x-www-form-urlencoded"},
249+
}
250+
251+
for i, tt := range tests {
252+
t.Run(fmt.Sprintf("test #%v", i), func(t *testing.T) {
253+
t.Parallel()
254+
// Simulate a reader that reports more than maxPayloadSize bytes.
255+
oversized := io.LimitReader(infiniteReader{}, maxPayloadSize+1)
256+
req := &http.Request{
257+
Header: http.Header{"Content-Type": []string{tt.contentType}},
258+
Body: io.NopCloser(oversized),
259+
}
260+
_, err := ValidatePayload(req, nil)
261+
if err == nil {
262+
t.Fatal("ValidatePayload returned nil; want error for oversized body")
263+
}
264+
if want := "webhook payload exceeds maximum allowed size"; err.Error() != want {
265+
t.Errorf("ValidatePayload error = %q, want %q", err.Error(), want)
266+
}
267+
})
268+
}
269+
}
270+
231271
func TestValidatePayload_InvalidContentTypeParams(t *testing.T) {
232272
t.Parallel()
233273
req, err := http.NewRequest("POST", "http://localhost/event", nil)

0 commit comments

Comments
 (0)