oci: casext: explicitly disallow negative-size descriptors

This was implicitly allowed by VerifiedReadCloser, and while we have
closed that hole it's probably best to provide a more helpful error
message.

Not blocking this earlier was mostly due to a somewhat overly-permissive
reading of the discussion in opencontainers/image-spec#153 which was
finally clarified in opencontainers/image-spec#1285. Unknown sizes are a
classic DoS vector, so allowing them (especially for descriptors where
it makes little sense to have an unknown size) seems like a bad idea in
general.

Ref: opencontainers/image-spec#1285
Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

CHANGELOG.md

@@ -19,6 +19,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   the `manifests` entry to `null` (which was technically a violation of the
   specification, though such images cannot be pushed or interacted with outside
   of umoci).
+* Based on [some recent developments in the image-spec][image-spec#1285], umoci
+  will now produce an error if it encounters descriptors with a negative size
+  (this was a potential DoS vector previously) as well as a theoretical attack
+  where an attacker would endlessly write to a blob (this would not be
+  generally exploitable for images with descriptors).
 
 ### Changed ###
 * We now use `go:embed` to fill the version information of `umoci --version`,
@@ -33,6 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   (and after that, we may choose to keep the `jq`-based validators as a
   double-check that our own validators are working correctly).
 
+[image-spec#1285]: https://github.com/opencontainers/image-spec/pull/1285
 [docker-library/meta-scripts]: https://github.com/docker-library/meta-scripts
 
 ## [0.5.0] - 2025-05-21 ##

oci/casext/verified_blob.go

@@ -20,19 +20,27 @@ package casext
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"io"
 
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 
 	"github.com/opencontainers/umoci/pkg/hardening"
 )
 
+var errInvalidDescriptorSize = errors.New("descriptor size must not be negative")
+
 // GetVerifiedBlob returns a VerifiedReadCloser for retrieving a blob from the
 // image, which the caller must Close() *and* read-to-EOF (checking the error
 // code of both). Returns ErrNotExist if the digest is not found, and
 // ErrBlobDigestMismatch on a mismatched blob digest. In addition, the reader
 // is limited to the descriptor.Size.
 func (e Engine) GetVerifiedBlob(ctx context.Context, descriptor ispec.Descriptor) (io.ReadCloser, error) {
+	// Negative sizes are not permitted by the spec, and are a DoS vector.
+	if descriptor.Size < 0 {
+		return nil, fmt.Errorf("invalid descriptor: %w", errInvalidDescriptorSize)
+	}
 	reader, err := e.GetBlob(ctx, descriptor.Digest)
 	return &hardening.VerifiedReadCloser{
 		Reader:         reader,

oci/casext/verified_blob_test.go

@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: Apache-2.0
+/*
+ * umoci: Umoci Modifies Open Containers' Images
+ * Copyright (C) 2016-2025 SUSE LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package casext
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"path/filepath"
+	"testing"
+
+	ispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/opencontainers/umoci/oci/cas/dir"
+)
+
+func TestGetVerifiedBlob(t *testing.T) {
+	image := filepath.Join(t.TempDir(), "image")
+	err := dir.Create(image)
+	require.NoError(t, err)
+
+	engine, err := dir.Open(image)
+	require.NoError(t, err)
+	engineExt := NewEngine(engine)
+	defer engine.Close() //nolint:errcheck
+
+	descMap := fakeSetupEngine(t, engineExt)
+	assert.NotEmpty(t, descMap, "fakeSetupEngine descriptor map")
+
+	t.Run("InvalidSize", func(t *testing.T) {
+		for idx, test := range descMap {
+			test := test // copy iterator
+			t.Run(fmt.Sprintf("Descriptor%.2d", idx+1), func(t *testing.T) {
+				ctx := context.Background()
+				desc := test.result
+
+				blob, err := engineExt.GetVerifiedBlob(ctx, desc)
+				assert.NoError(t, err, "get verified blob (regular descriptor)") //nolint:testifylint // assert.*Error* makes more sense
+				if assert.NotNil(t, blob, "get verified blob (regular descriptor)") {
+					// Avoid "trailing data" log warnings on Close.
+					_, _ = io.Copy(io.Discard, blob)
+					_ = blob.Close()
+				}
+
+				badDescriptor := ispec.Descriptor{
+					MediaType: desc.MediaType,
+					Digest:    desc.Digest,
+					Size:      -1, // invalid!
+				}
+
+				blob, err = engineExt.GetVerifiedBlob(ctx, badDescriptor)
+				assert.ErrorIs(t, err, errInvalidDescriptorSize, "get verified blob (negative descriptor size)") //nolint:testifylint // assert.*Error* makes more sense
+				if !assert.Nil(t, blob, "get verified blob (negative descriptor size)") {
+					_ = blob.Close()
+				}
+			})
+		}
+	})
+}