Skip to content

Commit 3ca2fdc

Browse files
committed
limit webhook payload size in ValidatePayloadFromBody
1 parent a276aa8 commit 3ca2fdc

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

github/messages.go

Lines changed: 12 additions & 2 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 (
@@ -199,9 +203,12 @@ func ValidatePayloadFromBody(contentType string, readable io.Reader, signature s
199203
switch contentType {
200204
case "application/json":
201205
var err error
202-
if body, err = io.ReadAll(readable); err != nil {
206+
if body, err = io.ReadAll(io.LimitReader(readable, maxPayloadSize+1)); err != nil {
203207
return nil, err
204208
}
209+
if int64(len(body)) > maxPayloadSize {
210+
return nil, errors.New("webhook payload exceeds maximum allowed size")
211+
}
205212

206213
// If the content type is application/json,
207214
// the JSON payload is just the original body.
@@ -213,9 +220,12 @@ func ValidatePayloadFromBody(contentType string, readable io.Reader, signature s
213220
const payloadFormParam = "payload"
214221

215222
var err error
216-
if body, err = io.ReadAll(readable); err != nil {
223+
if body, err = io.ReadAll(io.LimitReader(readable, maxPayloadSize+1)); err != nil {
217224
return nil, err
218225
}
226+
if int64(len(body)) > maxPayloadSize {
227+
return nil, errors.New("webhook payload exceeds maximum allowed size")
228+
}
219229

220230
// If the content type is application/x-www-form-urlencoded,
221231
// the JSON payload will be under the "payload" form param.

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)