diff --git a/oci/layer/unpack.go b/oci/layer/unpack.go index 1ac63a4e..2a714731 100644 --- a/oci/layer/unpack.go +++ b/oci/layer/unpack.go @@ -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 { diff --git a/oci/layer/unpack_test.go b/oci/layer/unpack_test.go index 03b5628b..5ae90903 100644 --- a/oci/layer/unpack_test.go +++ b/oci/layer/unpack_test.go @@ -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 // is resolved. func TestUnpackUnimplementedOverlayfs(t *testing.T) { diff --git a/utils.go b/utils.go index 7574a76e..ead5f609 100644 --- a/utils.go +++ b/utils.go @@ -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++