CVE-2024-34084: Insufficient Information (NVD-CWE-noinfo)
value: Minder's `HandleGithubWebhook` is susceptible to a denial of service attack from an untrusted HTTP request. The vulnerability exists before the request has been validated, and as such the request is still untrusted at the point of failure. This allows an attacker with the ability to send requests to `HandleGithubWebhook` to crash the Minder controlplane and deny other users from using it. This vulnerability is fixed in 0.0.48.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-07T15:15Z
Repository: https://github.com/stacklok/minder
Language: Go
Files changed: internal/controlplane/handlers_githubwebhooks_test.go, internal/controlplane/server.go
Commit date: 2024-05-07 10:05:45 +0300
Commit message: Merge pull request from GHSA-9c5w-9q3f-3hv7

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
Diff: diff --git a/internal/controlplane/handlers_githubwebhooks_test.go b/internal/controlplane/handlers_githubwebhooks_test.go
index 4b3c90aa6..bfec3ffbd 100644
--- a/internal/controlplane/handlers_githubwebhooks_test.go
+++ b/internal/controlplane/handlers_githubwebhooks_test.go
@@ -19,12 +19,14 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/cenkalti/backoff/v4"
 	"github.com/google/go-github/v61/github"
@@ -434,10 +436,77 @@ func (s *UnitTestSuite) TestNoopWebhookHandler() {
 
 	require.NoError(t, err, "failed to make request")
 	assert.Equal(t, http.StatusOK, resp.StatusCode, "unexpected status code")
 }
 
+func (s *UnitTestSuite) TestHandleWebHookWithTooLargeRequest() {
+	t := s.T()
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockStore := mockdb.NewMockStore(ctrl)
+	srv, evt := newDefaultServer(t, mockStore, nil)
+	defer evt.Close()
+
+	pq := testqueue.NewPassthroughQueue(t)
+	queued := pq.GetQueue()
+
+	evt.Register(events.TopicQueueEntityEvaluate, pq.Pass)
+
+	go func() {
+		err := evt.Run(context.Background())
+		require.NoError(t, err, "failed to run eventer")
+	}()
+
+	<-evt.Running()
+
+	hook := withMaxSizeMiddleware(srv.HandleGitHubWebHook())
+	port, err := rand.GetRandomPort()
+	if err != nil {
+		t.Fatal(err)
+	}
+	addr := fmt.Sprintf("localhost:%d", port)
+	server := &http.Server{
+		Addr:              fmt.Sprintf(":%d", port),
+		Handler:           hook,
+		ReadHeaderTimeout: 1 * time.Second,
+	}
+	go server.ListenAndServe()
+
+	event := github.PackageEvent{
+		Action: github.String("published"),
+		Repo: &github.Repository{
+			ID:   github.Int64(12345),
+			Name: github.String("stacklok/minder"),
+		},
+		Org: &github.Organization{
+			Login: github.String("stacklok"),
+		},
+	}
+	packageJson, err := json.Marshal(event)
+	require.NoError(t, err, "failed to marshal package event")
+
+	maliciousBody := strings.NewReader(strings.Repeat("1337", 1000000000))
+	maliciousBodyReader := io.MultiReader(maliciousBody, maliciousBody, maliciousBody, maliciousBody, maliciousBody)
+	_ = packageJson
+
+	client := &http.Client{}
+	req, err := http.NewRequest("POST", fmt.Sprintf("http://%s", addr), maliciousBodyReader)
+	require.NoError(t, err, "failed to create request")
+
+	req.Header.Add("X-GitHub-Event", "meta")
+	req.Header.Add("X-GitHub-Delivery", "12345")
+	req.Header.Add("Content-Type", "application/json")
+	resp, err := httpDoWithRetry(client, req)
+	require.NoError(t, err, "failed to make request")
+	// We expect OK since we don't want to leak information about registered repositories
+	require.Equal(t, http.StatusBadRequest, resp.StatusCode, "unexpected status code")
+	assert.Len(t, queued, 0)
+}
+
 func TestAll(t *testing.T) {
 	t.Parallel()
 
 	RunUnitTestSuite(t)
 	// Call other test runner functions for additional test suites
diff --git a/internal/controlplane/server.go b/internal/controlplane/server.go
index 1444905e0..3c4fc31f4 100644
--- a/internal/controlplane/server.go
+++ b/internal/controlplane/server.go
@@ -68,10 +68,14 @@ import (
 
 const metricsPath = "/metrics"
 
 var (
 	readHeaderTimeout = 2 * time.Second
+
+	// RequestBodyMaxBytes is the maximum number of bytes that can be read from a request body
+	// We limit to 2MB for now
+	RequestBodyMaxBytes int64 = 2 << 20
 )
 
 // Server represents the controlplane server
 type Server struct {
 	store               db.Store
@@ -314,14 +318,14 @@ func (s *Server) StartHTTPServer(ctx context.Context) error {
 	err = gwmux.HandlePath(http.MethodGet, "/api/v1/auth/callback/{provider}/app", s.HandleGitHubAppCallback())
 	if err != nil {
 		return fmt.Errorf("failed to register GitHub App callback handler: %w", err)
 	}
 
-	mux.Handle("/", s.handlerWithHTTPMiddleware(gwmux))
-	mux.Handle("/api/v1/webhook/", mw(s.HandleGitHubWebHook()))
-	mux.Handle("/api/v1/ghapp/", mw(s.HandleGitHubAppWebhook()))
-	mux.Handle("/api/v1/gh-marketplace/", mw(s.NoopWebhookHandler()))
+	mux.Handle("/", withMaxSizeMiddleware(s.handlerWithHTTPMiddleware(gwmux)))
+	mux.Handle("/api/v1/webhook/", mw(withMaxSizeMiddleware(s.HandleGitHubWebHook())))
+	mux.Handle("/api/v1/ghapp/", mw(withMaxSizeMiddleware(s.HandleGitHubAppWebhook())))
+	mux.Handle("/api/v1/gh-marketplace/", mw(withMaxSizeMiddleware(s.NoopWebhookHandler())))
 	mux.Handle("/static/", fs)
 
 	errch := make(chan error)
 
 	log.Printf("Starting HTTP server on %s", s.cfg.HTTPServer.GetAddress())
@@ -449,5 +453,12 @@ func shutdownHandler(component string, sdf shutdowner) {
 
 	if err := sdf(shutdownCtx); err != nil {
 		log.Fatal().Msgf("error shutting down '%s': %+v", component, err)
 	}
 }
+
+func withMaxSizeMiddleware(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		r.Body = http.MaxBytesReader(w, r.Body, RequestBodyMaxBytes)
+		h.ServeHTTP(w, r)
+	})
+}

Vulnerable code: 	mux.Handle("/", s.handlerWithHTTPMiddleware(gwmux))
	mux.Handle("/api/v1/webhook/", mw(s.HandleGitHubWebHook()))
	mux.Handle("/api/v1/ghapp/", mw(s.HandleGitHubAppWebhook()))
	mux.Handle("/api/v1/gh-marketplace/", mw(s.NoopWebhookHandler()))
Fixed code: 	"io"
	"strings"
func (s *UnitTestSuite) TestHandleWebHookWithTooLargeRequest() {
	t := s.T()
	t.Parallel()

	ctrl := gomock.NewController(t)
	defer ctrl.Finish()

	mockStore := mockdb.NewMockStore(ctrl)
	srv, evt := newDefaultServer(t, mockStore, nil)
	defer evt.Close()

	pq := testqueue.NewPassthroughQueue(t)
	queued := pq.GetQueue()

	evt.Register(events.TopicQueueEntityEvaluate, pq.Pass)

	go func() {
		err := evt.Run(context.Background())
		require.NoError(t, err, "failed to run eventer")
	}()

	<-evt.Running()

	hook := withMaxSizeMiddleware(srv.HandleGitHubWebHook())
	port, err := rand.GetRandomPort()
	if err != nil {
		t.Fatal(err)
	}
	addr := fmt.Sprintf("localhost:%d", port)
	server := &http.Server{
		Addr:              fmt.Sprintf(":%d", port),
		Handler:           hook,
		ReadHeaderTimeout: 1 * time.Second,
	}
	go server.ListenAndServe()

	event := github.PackageEvent{
		Action: github.String("published"),
		Repo: &github.Repository{
			ID:   github.Int64(12345),
			Name: github.String("stacklok/minder"),
		},
		Org: &github.Organization{
			Login: github.String("stacklok"),
		},
	}
	packageJson, err := json.Marshal(event)
	require.NoError(t, err, "failed to marshal package event")

	maliciousBody := strings.NewReader(strings.Repeat("1337", 1000000000))
	maliciousBodyReader := io.MultiReader(maliciousBody, maliciousBody, maliciousBody, maliciousBody, maliciousBody)
	_ = packageJson

	client := &http.Client{}
	req, err := http.NewRequest("POST", fmt.Sprintf("http://%s", addr), maliciousBodyReader)
	require.NoError(t, err, "failed to create request")

	req.Header.Add("X-GitHub-Event", "meta")
	req.Header.Add("X-GitHub-Delivery", "12345")
	req.Header.Add("Content-Type", "application/json")
	resp, err := httpDoWithRetry(client, req)
	require.NoError(t, err, "failed to make request")
	// We expect OK since we don't want to leak information about registered repositories
	require.Equal(t, http.StatusBadRequest, resp.StatusCode, "unexpected status code")
	assert.Len(t, queued, 0)
}


	// RequestBodyMaxBytes is the maximum number of bytes that can be read from a request body
	// We limit to 2MB for now
	RequestBodyMaxBytes int64 = 2 << 20
	mux.Handle("/", withMaxSizeMiddleware(s.handlerWithHTTPMiddleware(gwmux)))
	mux.Handle("/api/v1/webhook/", mw(withMaxSizeMiddleware(s.HandleGitHubWebHook())))
	mux.Handle("/api/v1/ghapp/", mw(withMaxSizeMiddleware(s.HandleGitHubAppWebhook())))
	mux.Handle("/api/v1/gh-marketplace/", mw(withMaxSizeMiddleware(s.NoopWebhookHandler())))

func withMaxSizeMiddleware(h http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		r.Body = http.MaxBytesReader(w, r.Body, RequestBodyMaxBytes)
		h.ServeHTTP(w, r)
	})
}