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 <rchincha.dev@gmail.com>

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"

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)

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{

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": {

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": {

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: