From 10ade61de947ac08c6d39d417c88bb5dd3cd2c89 Mon Sep 17 00:00:00 2001 From: Samuel Karp Date: Thu, 21 Oct 2021 15:09:39 -0700 Subject: [PATCH] manifest: validate document type before unmarshal Signed-off-by: Samuel Karp --- manifest/manifestlist/manifestlist.go | 23 ++++++++++++++++ manifest/manifestlist/manifestlist_test.go | 32 ++++++++++++++++++++++ manifest/ocischema/manifest.go | 22 +++++++++++++++ manifest/ocischema/manifest_test.go | 32 ++++++++++++++++++++++ 4 files changed, 109 insertions(+) diff --git a/manifest/manifestlist/manifestlist.go b/manifest/manifestlist/manifestlist.go index 3a1d73e83..bea2341c7 100644 --- a/manifest/manifestlist/manifestlist.go +++ b/manifest/manifestlist/manifestlist.go @@ -54,6 +54,9 @@ func init() { } imageIndexFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { + if err := validateIndex(b); err != nil { + return nil, distribution.Descriptor{}, err + } m := new(DeserializedManifestList) err := m.UnmarshalJSON(b) if err != nil { @@ -214,3 +217,23 @@ func (m DeserializedManifestList) Payload() (string, []byte, error) { return mediaType, m.canonical, nil } + +// unknownDocument represents a manifest, manifest list, or index that has not +// yet been validated +type unknownDocument struct { + Config interface{} `json:"config,omitempty"` + Layers interface{} `json:"layers,omitempty"` +} + +// validateIndex returns an error if the byte slice is invalid JSON or if it +// contains fields that belong to a manifest +func validateIndex(b []byte) error { + var doc unknownDocument + if err := json.Unmarshal(b, &doc); err != nil { + return err + } + if doc.Config != nil || doc.Layers != nil { + return errors.New("index: expected index but found manifest") + } + return nil +} diff --git a/manifest/manifestlist/manifestlist_test.go b/manifest/manifestlist/manifestlist_test.go index 6d0d952c2..f755921bf 100644 --- a/manifest/manifestlist/manifestlist_test.go +++ b/manifest/manifestlist/manifestlist_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/docker/distribution" + "github.com/docker/distribution/manifest/ocischema" + v1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -303,3 +305,33 @@ func TestMediaTypes(t *testing.T) { mediaTypeTest(t, v1.MediaTypeImageIndex, v1.MediaTypeImageIndex, false) mediaTypeTest(t, v1.MediaTypeImageIndex, v1.MediaTypeImageIndex+"XXX", true) } + +func TestValidateManifest(t *testing.T) { + manifest := ocischema.Manifest{ + Config: distribution.Descriptor{Size: 1}, + Layers: []distribution.Descriptor{{Size: 2}}, + } + index := ManifestList{ + Manifests: []ManifestDescriptor{ + {Descriptor: distribution.Descriptor{Size: 3}}, + }, + } + t.Run("valid", func(t *testing.T) { + b, err := json.Marshal(index) + if err != nil { + t.Fatal("unexpected error marshaling index", err) + } + if err := validateIndex(b); err != nil { + t.Error("index should be valid", err) + } + }) + t.Run("invalid", func(t *testing.T) { + b, err := json.Marshal(manifest) + if err != nil { + t.Fatal("unexpected error marshaling manifest", err) + } + if err := validateIndex(b); err == nil { + t.Error("manifest should not be valid") + } + }) +} diff --git a/manifest/ocischema/manifest.go b/manifest/ocischema/manifest.go index c5e85f285..d51f8debb 100644 --- a/manifest/ocischema/manifest.go +++ b/manifest/ocischema/manifest.go @@ -22,6 +22,9 @@ var ( func init() { ocischemaFunc := func(b []byte) (distribution.Manifest, distribution.Descriptor, error) { + if err := validateManifest(b); err != nil { + return nil, distribution.Descriptor{}, err + } m := new(DeserializedManifest) err := m.UnmarshalJSON(b) if err != nil { @@ -122,3 +125,22 @@ func (m *DeserializedManifest) MarshalJSON() ([]byte, error) { func (m DeserializedManifest) Payload() (string, []byte, error) { return v1.MediaTypeImageManifest, m.canonical, nil } + +// unknownDocument represents a manifest, manifest list, or index that has not +// yet been validated +type unknownDocument struct { + Manifests interface{} `json:"manifests,omitempty"` +} + +// validateManifest returns an error if the byte slice is invalid JSON or if it +// contains fields that belong to a index +func validateManifest(b []byte) error { + var doc unknownDocument + if err := json.Unmarshal(b, &doc); err != nil { + return err + } + if doc.Manifests != nil { + return errors.New("ocimanifest: expected manifest but found index") + } + return nil +} diff --git a/manifest/ocischema/manifest_test.go b/manifest/ocischema/manifest_test.go index b211fd094..2b6fc5bcd 100644 --- a/manifest/ocischema/manifest_test.go +++ b/manifest/ocischema/manifest_test.go @@ -8,6 +8,8 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/manifest" + "github.com/docker/distribution/manifest/manifestlist" + v1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -182,3 +184,33 @@ func TestMediaTypes(t *testing.T) { mediaTypeTest(t, v1.MediaTypeImageManifest, false) mediaTypeTest(t, v1.MediaTypeImageManifest+"XXX", true) } + +func TestValidateManifest(t *testing.T) { + manifest := Manifest{ + Config: distribution.Descriptor{Size: 1}, + Layers: []distribution.Descriptor{{Size: 2}}, + } + index := manifestlist.ManifestList{ + Manifests: []manifestlist.ManifestDescriptor{ + {Descriptor: distribution.Descriptor{Size: 3}}, + }, + } + t.Run("valid", func(t *testing.T) { + b, err := json.Marshal(manifest) + if err != nil { + t.Fatal("unexpected error marshaling manifest", err) + } + if err := validateManifest(b); err != nil { + t.Error("manifest should be valid", err) + } + }) + t.Run("invalid", func(t *testing.T) { + b, err := json.Marshal(index) + if err != nil { + t.Fatal("unexpected error marshaling index", err) + } + if err := validateManifest(b); err == nil { + t.Error("index should not be valid") + } + }) +}