From 35c29b95e4452d7b3c5c8fcca259168ee83dcbe1 Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani <45800463+rchincha@users.noreply.github.com> Date: Fri, 17 Apr 2026 13:10:51 -0700 Subject: [PATCH] fix(security): limit manifest PUT body to 4 MiB (INPUT-1) (#3977) Wrap request.Body with http.MaxBytesReader before io.ReadAll in UpdateManifest. Bodies exceeding MaxManifestBodySize (4 MiB) now return HTTP 413 with a MANIFEST_INVALID error body instead of buffering unlimited data into memory. Add the MaxManifestBodySize constant and a unit test that sends an oversized body and asserts the 413 status. Agent-Logs-Url: https://github.com/project-zot/zot/sessions/5eca86eb-9749-4cf8-9fb8-7b9ace2ba87f Signed-off-by: Ramkumar Chinchani --- pkg/api/constants/consts.go | 5 ++++- pkg/api/routes.go | 29 +++++++++++++++++++---------- pkg/api/routes_test.go | 22 ++++++++++++++++++++++ swagger/docs.go | 6 ++++++ swagger/swagger.json | 6 ++++++ swagger/swagger.yaml | 4 ++++ 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/pkg/api/constants/consts.go b/pkg/api/constants/consts.go index 1afa9ad2..93c00883 100644 --- a/pkg/api/constants/consts.go +++ b/pkg/api/constants/consts.go @@ -19,7 +19,10 @@ const ( // for path and digest: // // (8192 - 2048) / (len("tag=") + 128 + 1) == 46 - MaxManifestDigestQueryTags = (8192 - 2048) / (len("tag=") + 128 + 1) + MaxManifestDigestQueryTags = (8192 - 2048) / (len("tag=") + 128 + 1) + // MaxManifestBodySize is the maximum number of bytes accepted for a manifest PUT request body. + // OCI manifest JSON is always small metadata; 4 MiB is well above any realistic manifest. + MaxManifestBodySize = 4 * 1024 * 1024 BlobUploadUUID = "Blob-Upload-UUID" DefaultMediaType = "application/json" BinaryMediaType = "application/octet-stream" diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 91b1937e..5c2e431f 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -689,6 +689,7 @@ func (rh *RouteHandler) GetReferrers(response http.ResponseWriter, request *http // @Header 201 {string} OCI-Tag "Echoed tag= value; this header is repeatable (one field per tag= query parameter)" // @Failure 400 {string} string "bad request" // @Failure 404 {string} string "not found" +// @Failure 413 {string} string "request entity too large" // @Failure 414 {string} string "too many tag query parameters" // @Failure 500 {string} string "internal server error" // @Router /v2/{name}/manifests/{reference} [put]. @@ -746,16 +747,6 @@ func (rh *RouteHandler) UpdateManifest(response http.ResponseWriter, request *ht } } - body, err := io.ReadAll(request.Body) - // hard to reach test case, injected error (simulates an interrupted image manifest upload) - // err could be io.ErrUnexpectedEOF - if err := inject.Error(err); err != nil { - rh.c.Log.Error().Err(err).Msg("unexpected error") - response.WriteHeader(http.StatusInternalServerError) - - return - } - if len(digestQueryTags) > 0 && !zcommon.IsDigest(reference) { err := apiErr.NewError(apiErr.MANIFEST_INVALID).AddDetail(map[string]string{ "reason": "tag query parameters are only valid when pushing a manifest by digest", @@ -765,6 +756,24 @@ func (rh *RouteHandler) UpdateManifest(response http.ResponseWriter, request *ht return } + body, err := io.ReadAll(http.MaxBytesReader(response, request.Body, constants.MaxManifestBodySize)) + // hard to reach test case, injected error (simulates an interrupted image manifest upload) + // err could be io.ErrUnexpectedEOF or *http.MaxBytesError + if err := inject.Error(err); err != nil { + var mbe *http.MaxBytesError + if errors.As(err, &mbe) { + e := apiErr.NewError(apiErr.MANIFEST_INVALID).AddDetail(map[string]string{ + "reason": fmt.Sprintf("manifest body exceeds maximum allowed size of %d bytes", constants.MaxManifestBodySize), + }) + zcommon.WriteJSON(response, http.StatusRequestEntityTooLarge, apiErr.NewErrorList(e)) + } else { + rh.c.Log.Error().Err(err).Msg("unexpected error") + response.WriteHeader(http.StatusInternalServerError) + } + + return + } + digest, subjectDigest, err := imgStore.PutImageManifest(name, reference, mediaType, body, digestQueryTags) if err != nil { details := zerr.GetDetails(err) diff --git a/pkg/api/routes_test.go b/pkg/api/routes_test.go index 119ca488..d65af6fe 100644 --- a/pkg/api/routes_test.go +++ b/pkg/api/routes_test.go @@ -26,6 +26,7 @@ import ( "zotregistry.dev/zot/v2/pkg/api" "zotregistry.dev/zot/v2/pkg/api/config" "zotregistry.dev/zot/v2/pkg/api/constants" + apiErr "zotregistry.dev/zot/v2/pkg/api/errors" "zotregistry.dev/zot/v2/pkg/log" mTypes "zotregistry.dev/zot/v2/pkg/meta/types" reqCtx "zotregistry.dev/zot/v2/pkg/requestcontext" @@ -259,6 +260,27 @@ func TestRoutes(t *testing.T) { return resp.StatusCode } + + Convey("body exceeds MaxManifestBodySize returns 413 with MANIFEST_INVALID error payload", func() { + ctlr.StoreController.DefaultStore = &mocks.MockedImageStore{} + oversized := make([]byte, constants.MaxManifestBodySize+1) + request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, baseURL, + bytes.NewReader(oversized)) + request = mux.SetURLVars(request, map[string]string{"name": "test", "reference": "v1"}) + request.Header.Add("Content-Type", ispec.MediaTypeImageManifest) + response := httptest.NewRecorder() + + rthdlr.UpdateManifest(response, request) + + So(response.Code, ShouldEqual, http.StatusRequestEntityTooLarge) + + var errList apiErr.ErrorList + err := json.NewDecoder(response.Body).Decode(&errList) + So(err, ShouldBeNil) + So(errList.Errors, ShouldHaveLength, 1) + So(errList.Errors[0].Code, ShouldEqual, apiErr.MANIFEST_INVALID.String()) + So(errList.Errors[0].Detail["reason"], ShouldContainSubstring, "exceeds maximum allowed size") + }) // repo not found statusCode := testUpdateManifest( map[string]string{ diff --git a/swagger/docs.go b/swagger/docs.go index fb2c45c3..af166c0b 100644 --- a/swagger/docs.go +++ b/swagger/docs.go @@ -786,6 +786,12 @@ const docTemplate = `{ "type": "string" } }, + "413": { + "description": "request entity too large", + "schema": { + "type": "string" + } + }, "414": { "description": "too many tag query parameters", "schema": { diff --git a/swagger/swagger.json b/swagger/swagger.json index 247f95fa..15077763 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -778,6 +778,12 @@ "type": "string" } }, + "413": { + "description": "request entity too large", + "schema": { + "type": "string" + } + }, "414": { "description": "too many tag query parameters", "schema": { diff --git a/swagger/swagger.yaml b/swagger/swagger.yaml index 09b30dcc..6bffaab4 100644 --- a/swagger/swagger.yaml +++ b/swagger/swagger.yaml @@ -840,6 +840,10 @@ paths: description: not found schema: type: string + "413": + description: request entity too large + schema: + type: string "414": description: too many tag query parameters schema: