From 0296b30124eec93ead90524b1893846fa814d6f8 Mon Sep 17 00:00:00 2001 From: Marc Campbell Date: Mon, 27 Jul 2026 14:37:23 -0600 Subject: [PATCH 1/2] fix: bound network response body reads Limit allocation on Vendor API, login token, CMX metadata, checksum, and OCI manifest response bodies with generous ceilings that preserve normal use. --- cli/cmd/model_pull.go | 8 ++++++-- pkg/cmxmetadata/cmxmetadata.go | 17 +++++++++++++++-- pkg/credentials/fetch.go | 7 ++++++- pkg/platformclient/client.go | 27 +++++++++++++++++++++------ pkg/platformclient/client_test.go | 17 +++++++++++++++++ pkg/tools/checksum.go | 18 ++++++++++++++++-- pkg/tools/checksum_limit_test.go | 23 +++++++++++++++++++++++ 7 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 pkg/tools/checksum_limit_test.go diff --git a/cli/cmd/model_pull.go b/cli/cmd/model_pull.go index 9b8486668..aa9b7c091 100644 --- a/cli/cmd/model_pull.go +++ b/cli/cmd/model_pull.go @@ -109,11 +109,15 @@ func (r *runners) pullModel(cmd *cobra.Command, args []string) error { } defer manifestContent.Close() - // Read the manifest content into a byte slice - manifestBytes, err := io.ReadAll(manifestContent) + // Read the manifest content into a byte slice (OCI manifests are small JSON). + const maxManifestBody = 16 << 20 // 16 MiB + manifestBytes, err := io.ReadAll(io.LimitReader(manifestContent, maxManifestBody+1)) if err != nil { return err } + if len(manifestBytes) > maxManifestBody { + return fmt.Errorf("OCI manifest exceeds %d bytes", maxManifestBody) + } var manifest v1.Manifest if err := json.Unmarshal(manifestBytes, &manifest); err != nil { diff --git a/pkg/cmxmetadata/cmxmetadata.go b/pkg/cmxmetadata/cmxmetadata.go index 6e261bbec..e5bcb8cef 100644 --- a/pkg/cmxmetadata/cmxmetadata.go +++ b/pkg/cmxmetadata/cmxmetadata.go @@ -17,6 +17,8 @@ const ( mmdsPath = "/latest/vendor-api" mmdsTimeout = 500 * time.Millisecond // fail fast if not in CMX tokenLeeway = 60 * time.Second // refresh token this early before expiry + // maxMetadataBody bounds MMDS / token JSON responses (credentials, not archives). + maxMetadataBody = 1 << 20 // 1 MiB ) // ErrNotAvailable is returned when the CMX metadata service is not reachable. @@ -61,7 +63,7 @@ func GetVMMetadata() (*VMMetadata, error) { return nil, ErrNotAvailable } - body, err := io.ReadAll(resp.Body) + body, err := readAllLimited(resp.Body, maxMetadataBody) if err != nil { return nil, ErrNotAvailable } @@ -140,7 +142,7 @@ func exchangeCredentials(meta *VMMetadata) (string, *time.Time, error) { } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := readAllLimited(resp.Body, maxMetadataBody) if err != nil { return "", nil, fmt.Errorf("reading token response body: %w", err) } @@ -166,3 +168,14 @@ func exchangeCredentials(meta *VMMetadata) (string, *time.Time, error) { return tokenResp.AccessToken, expiresAt, nil } + +func readAllLimited(r io.Reader, limit int64) ([]byte, error) { + body, err := io.ReadAll(io.LimitReader(r, limit+1)) + if err != nil { + return nil, err + } + if int64(len(body)) > limit { + return nil, fmt.Errorf("response body exceeds %d bytes", limit) + } + return body, nil +} diff --git a/pkg/credentials/fetch.go b/pkg/credentials/fetch.go index 8557e471f..a511d7982 100644 --- a/pkg/credentials/fetch.go +++ b/pkg/credentials/fetch.go @@ -122,10 +122,15 @@ func exchangeNonceForToken(uri string, nonce string) (string, error) { return "", fmt.Errorf("unexpected status code: %d", resp.StatusCode) } - b, err := io.ReadAll(resp.Body) + // Token JSON is small; bound allocation against a hostile exchange endpoint. + const maxTokenResponse = 1 << 20 // 1 MiB + b, err := io.ReadAll(io.LimitReader(resp.Body, maxTokenResponse+1)) if err != nil { return "", err } + if len(b) > maxTokenResponse { + return "", fmt.Errorf("token response exceeds %d bytes", maxTokenResponse) + } type tokenResponse struct { Token string `json:"token"` diff --git a/pkg/platformclient/client.go b/pkg/platformclient/client.go index 5ac3dc6b4..498dfab9e 100644 --- a/pkg/platformclient/client.go +++ b/pkg/platformclient/client.go @@ -19,6 +19,10 @@ import ( const apiOrigin = "https://api.replicated.com/vendor" +// maxResponseBody is a generous ceiling for Vendor API response bodies. +// Large multi-document releases must still fit; this only bounds unbounded allocation. +const maxResponseBody = 100 << 20 // 100 MiB + var ( ErrForbidden = errors.New("the action is not allowed for the current user or team") ) @@ -155,7 +159,7 @@ func (c *HTTPClient) DoJSONWithoutUnmarshal(ctx context.Context, method string, } defer resp.Body.Close() - bodyBytes, err := io.ReadAll(resp.Body) + bodyBytes, err := readAllLimited(resp.Body, maxResponseBody) if err != nil { return nil, errors.Wrap(err, "read body") } @@ -212,14 +216,14 @@ func (c *HTTPClient) DoJSON(ctx context.Context, method string, path string, suc } if resp.StatusCode != successStatus { if resp.StatusCode == http.StatusForbidden { - body, err := io.ReadAll(resp.Body) + body, err := readAllLimited(resp.Body, maxResponseBody) if err != nil { return ErrForbidden } return parseForbiddenError(body) } - body, _ := io.ReadAll(resp.Body) + body, _ := readAllLimited(resp.Body, maxResponseBody) return APIError{ Method: method, Endpoint: endpoint, @@ -229,7 +233,7 @@ func (c *HTTPClient) DoJSON(ctx context.Context, method string, path string, suc } } if respBody != nil { - bodyBytes, err := io.ReadAll(resp.Body) + bodyBytes, err := readAllLimited(resp.Body, maxResponseBody) if err != nil { return errors.Wrap(err, "read body") } @@ -242,6 +246,17 @@ func (c *HTTPClient) DoJSON(ctx context.Context, method string, path string, suc return nil } +func readAllLimited(r io.Reader, limit int64) ([]byte, error) { + body, err := io.ReadAll(io.LimitReader(r, limit+1)) + if err != nil { + return nil, err + } + if int64(len(body)) > limit { + return nil, fmt.Errorf("response body exceeds %d bytes", limit) + } + return body, nil +} + func (c *HTTPClient) setCommonHeaders(req *http.Request) error { req.Header.Set("Authorization", c.apiKey) req.Header.Set("Content-Type", "application/json") @@ -329,11 +344,11 @@ func (c *HTTPClient) HTTPGet(path string, successStatus int) ([]byte, error) { return nil, ErrNotFound } if resp.StatusCode != successStatus { - body, _ := io.ReadAll(resp.Body) + body, _ := readAllLimited(resp.Body, maxResponseBody) return nil, fmt.Errorf("GET %s %d: %s", endpoint, resp.StatusCode, body) } - return io.ReadAll(resp.Body) + return readAllLimited(resp.Body, maxResponseBody) } var knownErrorCodes = map[string]string{ diff --git a/pkg/platformclient/client_test.go b/pkg/platformclient/client_test.go index ba5cc9901..3a0d93d36 100644 --- a/pkg/platformclient/client_test.go +++ b/pkg/platformclient/client_test.go @@ -156,6 +156,23 @@ func TestDoJSONWithoutUnmarshalErrorHandlingAndHeaders(t *testing.T) { } } +func TestReadAllLimited(t *testing.T) { + t.Parallel() + + ok, err := readAllLimited(strings.NewReader("hello"), 10) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(ok) != "hello" { + t.Fatalf("got %q, want hello", ok) + } + + _, err = readAllLimited(strings.NewReader(strings.Repeat("a", 11)), 10) + if err == nil { + t.Fatal("expected error when body exceeds limit") + } +} + type roundTripFunc func(*http.Request) (*http.Response, error) func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { diff --git a/pkg/tools/checksum.go b/pkg/tools/checksum.go index 90f2ad2fe..844213f2d 100644 --- a/pkg/tools/checksum.go +++ b/pkg/tools/checksum.go @@ -15,6 +15,9 @@ var checksumHTTPClient = &http.Client{ Timeout: 30 * time.Second, } +// maxChecksumBody bounds checksum text files (not the archives themselves). +const maxChecksumBody = 1 << 20 // 1 MiB + // VerifyHelmChecksum verifies a Helm binary against its .sha256sum file func VerifyHelmChecksum(data []byte, archiveURL string) error { // Helm provides per-file checksums: .sha256sum @@ -31,7 +34,7 @@ func VerifyHelmChecksum(data []byte, archiveURL string) error { return fmt.Errorf("checksum file not found (HTTP %d): %s", resp.StatusCode, checksumURL) } - checksumData, err := io.ReadAll(resp.Body) + checksumData, err := readAllLimited(resp.Body, maxChecksumBody) if err != nil { return fmt.Errorf("reading checksum file: %w", err) } @@ -71,7 +74,7 @@ func VerifyTroubleshootChecksum(data []byte, version, filename string) error { return fmt.Errorf("checksums file not found (HTTP %d): %s", resp.StatusCode, checksumURL) } - checksumData, err := io.ReadAll(resp.Body) + checksumData, err := readAllLimited(resp.Body, maxChecksumBody) if err != nil { return fmt.Errorf("reading checksums file: %w", err) } @@ -102,3 +105,14 @@ func VerifyTroubleshootChecksum(data []byte, version, filename string) error { return nil } + +func readAllLimited(r io.Reader, limit int64) ([]byte, error) { + body, err := io.ReadAll(io.LimitReader(r, limit+1)) + if err != nil { + return nil, err + } + if int64(len(body)) > limit { + return nil, fmt.Errorf("response body exceeds %d bytes", limit) + } + return body, nil +} diff --git a/pkg/tools/checksum_limit_test.go b/pkg/tools/checksum_limit_test.go new file mode 100644 index 000000000..0a6a06b5f --- /dev/null +++ b/pkg/tools/checksum_limit_test.go @@ -0,0 +1,23 @@ +package tools + +import ( + "strings" + "testing" +) + +func TestReadAllLimited(t *testing.T) { + t.Parallel() + + got, err := readAllLimited(strings.NewReader("abc"), 10) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != "abc" { + t.Fatalf("got %q, want abc", got) + } + + _, err = readAllLimited(strings.NewReader(strings.Repeat("x", 5)), 4) + if err == nil { + t.Fatal("expected error when body exceeds limit") + } +} From 5b688b6c52d3da4480da8ecd939d15c81ac911a3 Mon Sep 17 00:00:00 2001 From: Marc Campbell Date: Mon, 27 Jul 2026 15:29:16 -0600 Subject: [PATCH 2/2] fix: drop Vendor API response body size limits Per review: this is a short-lived CLI talking to a trusted Vendor API. Hard caps can turn successful large responses into client errors without real security benefit. Keep limits only on small fixed-shape payloads. --- pkg/platformclient/client.go | 27 ++++++--------------------- pkg/platformclient/client_test.go | 17 ----------------- 2 files changed, 6 insertions(+), 38 deletions(-) diff --git a/pkg/platformclient/client.go b/pkg/platformclient/client.go index 498dfab9e..5ac3dc6b4 100644 --- a/pkg/platformclient/client.go +++ b/pkg/platformclient/client.go @@ -19,10 +19,6 @@ import ( const apiOrigin = "https://api.replicated.com/vendor" -// maxResponseBody is a generous ceiling for Vendor API response bodies. -// Large multi-document releases must still fit; this only bounds unbounded allocation. -const maxResponseBody = 100 << 20 // 100 MiB - var ( ErrForbidden = errors.New("the action is not allowed for the current user or team") ) @@ -159,7 +155,7 @@ func (c *HTTPClient) DoJSONWithoutUnmarshal(ctx context.Context, method string, } defer resp.Body.Close() - bodyBytes, err := readAllLimited(resp.Body, maxResponseBody) + bodyBytes, err := io.ReadAll(resp.Body) if err != nil { return nil, errors.Wrap(err, "read body") } @@ -216,14 +212,14 @@ func (c *HTTPClient) DoJSON(ctx context.Context, method string, path string, suc } if resp.StatusCode != successStatus { if resp.StatusCode == http.StatusForbidden { - body, err := readAllLimited(resp.Body, maxResponseBody) + body, err := io.ReadAll(resp.Body) if err != nil { return ErrForbidden } return parseForbiddenError(body) } - body, _ := readAllLimited(resp.Body, maxResponseBody) + body, _ := io.ReadAll(resp.Body) return APIError{ Method: method, Endpoint: endpoint, @@ -233,7 +229,7 @@ func (c *HTTPClient) DoJSON(ctx context.Context, method string, path string, suc } } if respBody != nil { - bodyBytes, err := readAllLimited(resp.Body, maxResponseBody) + bodyBytes, err := io.ReadAll(resp.Body) if err != nil { return errors.Wrap(err, "read body") } @@ -246,17 +242,6 @@ func (c *HTTPClient) DoJSON(ctx context.Context, method string, path string, suc return nil } -func readAllLimited(r io.Reader, limit int64) ([]byte, error) { - body, err := io.ReadAll(io.LimitReader(r, limit+1)) - if err != nil { - return nil, err - } - if int64(len(body)) > limit { - return nil, fmt.Errorf("response body exceeds %d bytes", limit) - } - return body, nil -} - func (c *HTTPClient) setCommonHeaders(req *http.Request) error { req.Header.Set("Authorization", c.apiKey) req.Header.Set("Content-Type", "application/json") @@ -344,11 +329,11 @@ func (c *HTTPClient) HTTPGet(path string, successStatus int) ([]byte, error) { return nil, ErrNotFound } if resp.StatusCode != successStatus { - body, _ := readAllLimited(resp.Body, maxResponseBody) + body, _ := io.ReadAll(resp.Body) return nil, fmt.Errorf("GET %s %d: %s", endpoint, resp.StatusCode, body) } - return readAllLimited(resp.Body, maxResponseBody) + return io.ReadAll(resp.Body) } var knownErrorCodes = map[string]string{ diff --git a/pkg/platformclient/client_test.go b/pkg/platformclient/client_test.go index 3a0d93d36..ba5cc9901 100644 --- a/pkg/platformclient/client_test.go +++ b/pkg/platformclient/client_test.go @@ -156,23 +156,6 @@ func TestDoJSONWithoutUnmarshalErrorHandlingAndHeaders(t *testing.T) { } } -func TestReadAllLimited(t *testing.T) { - t.Parallel() - - ok, err := readAllLimited(strings.NewReader("hello"), 10) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if string(ok) != "hello" { - t.Fatalf("got %q, want hello", ok) - } - - _, err = readAllLimited(strings.NewReader(strings.Repeat("a", 11)), 10) - if err == nil { - t.Fatal("expected error when body exceeds limit") - } -} - type roundTripFunc func(*http.Request) (*http.Response, error) func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) {