fix: Limit webhook payload size in ValidatePayloadFromBody (#4125)

Author: YooLCD

github/messages.go

@@ -42,6 +42,10 @@ const (
 	EventTypeHeader = "X-Github-Event"
 	// DeliveryIDHeader is the GitHub header key used to pass the unique ID for the webhook event.
 	DeliveryIDHeader = "X-Github-Delivery"
+
+	// maxPayloadSize is the maximum size of a GitHub webhook payload.
+	// GitHub documents a 25 MB limit for webhook payloads.
+	maxPayloadSize = 25 * 1024 * 1024
 )
 
 var (
@@ -146,8 +150,19 @@ func checkMAC(message, messageMAC, key []byte, hashFunc func() hash.Hash) bool {
 	return hmac.Equal(messageMAC, expectedMAC)
 }
 
-// messageMAC returns the hex-decoded HMAC tag from the signature and its
-// corresponding hash function.
+// readPayloadBody reads the body from readable, enforcing maxPayloadSize.
+func readPayloadBody(readable io.Reader) ([]byte, error) {
+	body, err := io.ReadAll(io.LimitReader(readable, maxPayloadSize+1))
+	if err != nil {
+		return nil, err
+	}
+	if len(body) > maxPayloadSize {
+		return nil, errors.New("webhook payload exceeds maximum allowed size")
+	}
+	return body, nil
+}
+
+// messageMAC returns the MAC method and the corresponding hash function.
 func messageMAC(signature string) ([]byte, func() hash.Hash, error) {
 	if signature == "" {
 		return nil, nil, errors.New("missing signature")
@@ -199,7 +214,7 @@ func ValidatePayloadFromBody(contentType string, readable io.Reader, signature s
 	switch contentType {
 	case "application/json":
 		var err error
-		if body, err = io.ReadAll(readable); err != nil {
+		if body, err = readPayloadBody(readable); err != nil {
 			return nil, err
 		}
 
@@ -213,7 +228,7 @@ func ValidatePayloadFromBody(contentType string, readable io.Reader, signature s
 		const payloadFormParam = "payload"
 
 		var err error
-		if body, err = io.ReadAll(readable); err != nil {
+		if body, err = readPayloadBody(readable); err != nil {
 			return nil, err
 		}
 

github/messages_test.go

@@ -10,6 +10,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 	"strings"
@@ -205,6 +206,10 @@ func (b *badReader) Read([]byte) (int, error) {
 
 func (b *badReader) Close() error { return errors.New("bad reader") }
 
+type readerFunc func([]byte) (int, error)
+
+func (f readerFunc) Read(p []byte) (int, error) { return f(p) }
+
 func TestValidatePayload_BadRequestBody(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
@@ -228,6 +233,40 @@ func TestValidatePayload_BadRequestBody(t *testing.T) {
 	}
 }
 
+func TestValidatePayload_OversizedBody(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		contentType string
+	}{
+		{contentType: "application/json"},
+		{contentType: "application/x-www-form-urlencoded"},
+	}
+
+	for i, tt := range tests {
+		t.Run(fmt.Sprintf("test #%v", i), func(t *testing.T) {
+			t.Parallel()
+			// Simulate a reader that reports more than maxPayloadSize bytes.
+			oversized := io.LimitReader(readerFunc(func(p []byte) (int, error) {
+				for i := range p {
+					p[i] = 0
+				}
+				return len(p), nil
+			}), maxPayloadSize+1)
+			req := &http.Request{
+				Header: http.Header{"Content-Type": []string{tt.contentType}},
+				Body:   io.NopCloser(oversized),
+			}
+			_, err := ValidatePayload(req, nil)
+			if err == nil {
+				t.Fatal("ValidatePayload returned nil; want error for oversized body")
+			}
+			if want := "webhook payload exceeds maximum allowed size"; err.Error() != want {
+				t.Errorf("ValidatePayload error = %q, want %q", err.Error(), want)
+			}
+		})
+	}
+}
+
 func TestValidatePayload_InvalidContentTypeParams(t *testing.T) {
 	t.Parallel()
 	req, err := http.NewRequest("POST", "http://localhost/event", nil)