Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions oci/layer/unpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,13 @@ func UnpackRootfs(ctx context.Context, engine cas.Engine, rootfsPath string, man
return fmt.Errorf("unpack rootfs: config: unsupported rootfs.type: %s", config.RootFS.Type)
}

// Both of these slices come from the untrusted image, so we cannot assume
// they line up. Bail out with a clear error rather than letting the loop
// below index config.RootFS.DiffIDs out of range and panic.
if len(manifest.Layers) != len(config.RootFS.DiffIDs) {
return fmt.Errorf("unpack rootfs: config: number of diff_ids (%d) does not match number of manifest layers (%d)", len(config.RootFS.DiffIDs), len(manifest.Layers))
}

// Layer extraction.
found := false
for idx, layerDescriptor := range manifest.Layers {
Expand Down
47 changes: 47 additions & 0 deletions oci/layer/unpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,53 @@ func TestUnpackStartFromDescriptor(t *testing.T) {
assert.ErrorIs(t, err, os.ErrNotExist, "test file should not be present")
}

// A crafted image whose manifest lists more layers than the config has
// diff_ids (both come from the untrusted image) used to walk off the end of
// config.RootFS.DiffIDs and panic. Make sure we return an error instead.
func TestUnpackManifestLayerDiffIDMismatch(t *testing.T) {
_, manifest, engineExt := makeImage(t)
require.Greater(t, len(manifest.Layers), 1, "test image needs at least two layers")

// Grab the original config so we can build a corrupted variant with fewer
// diff_ids than the manifest has layers.
configBlob, err := engineExt.FromDescriptor(t.Context(), manifest.Config)
require.NoError(t, err)
defer configBlob.Close() //nolint:errcheck // read-only blob
config, ok := configBlob.Data.(ispec.Image)
require.True(t, ok, "config blob should decode to an image")

// Drop the last diff_id so the counts no longer line up.
config.RootFS.DiffIDs = config.RootFS.DiffIDs[:len(config.RootFS.DiffIDs)-1]

configDigest, configSize, err := engineExt.PutBlobJSON(t.Context(), config)
require.NoError(t, err)
manifest.Config = ispec.Descriptor{
MediaType: ispec.MediaTypeImageConfig,
Digest: configDigest,
Size: configSize,
}

unpackOptions := &UnpackOptions{
OnDiskFormat: DirRootfs{
MapOptions: MapOptions{
UIDMappings: []rspec.LinuxIDMapping{
{HostID: uint32(os.Geteuid()), ContainerID: 0, Size: 1},
{HostID: uint32(os.Geteuid()), ContainerID: 1000, Size: 1},
},
GIDMappings: []rspec.LinuxIDMapping{
{HostID: uint32(os.Getegid()), ContainerID: 0, Size: 1},
{HostID: uint32(os.Getegid()), ContainerID: 100, Size: 1},
},
Rootless: os.Geteuid() != 0,
},
},
}
bundle := t.TempDir()
err = UnpackManifest(t.Context(), engineExt, bundle, manifest, unpackOptions)
require.Error(t, err, "UnpackManifest with mismatched diff_ids should fail cleanly")
assert.Contains(t, err.Error(), "does not match number of manifest layers")
}

// TODO: Temporary until <https://github.com/opencontainers/umoci/issues/574>
// is resolved.
func TestUnpackUnimplementedOverlayfs(t *testing.T) {
Expand Down
7 changes: 6 additions & 1 deletion utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,13 @@ func Stat(ctx context.Context, engine casext.Engine, manifestDescriptor ispec.De
Layer: nil,
}
// Only fill the other information and increment layerIdx if it's a
// non-empty layer.
// non-empty layer. Both slices come from the untrusted image, so a
// mismatch between the history and the diff_ids/layers must not be
// allowed to index out of range and panic.
if !histEntry.EmptyLayer {
if layerIdx >= len(config.RootFS.DiffIDs) || layerIdx >= len(manifest.Layers) {
return stat, fmt.Errorf("stat: config: number of non-empty history entries exceeds number of diff_ids (%d) or manifest layers (%d)", len(config.RootFS.DiffIDs), len(manifest.Layers))
}
info.DiffID = config.RootFS.DiffIDs[layerIdx]
info.Layer = &manifest.Layers[layerIdx]
layerIdx++
Expand Down
Loading