fix: Scrutinize references more closely

Signed-off-by: Nima Talebi <github@nima.id.au>

registry/reference.go

@@ -81,12 +81,12 @@ type Reference struct {
 // That is, of all the possible forms that `path` can take, there are exactly 4
 // that are considered valid.  Expanding the BNF notation for `path`, the 4 are:
 //
-//	<---------------------- path ----------------------> |  - Decode `path`
-//	<=== REPOSITORY ===><---------- reference ---------> |    - Decode `reference`
-//	<=== REPOSITORY ===><=================== digest ===> |      - Valid Form A
-//	<=== REPOSITORY ===><!!! TAG !!!> @ <=== digest ===> |      - Valid Form B
-//	<=== REPOSITORY ===><=== TAG ======================> |      - Valid Form C
-//	<=== REPOSITORY ===================================> |    - Valid Form D
+//	<--- path --------------------------------------------> |  - Decode `path`
+//	<=== REPOSITORY ===> <--- reference ------------------> |    - Decode `reference`
+//	<=== REPOSITORY ===> @ <=================== digest ===> |      - Valid Form A
+//	<=== REPOSITORY ===> : <!!! TAG !!!> @ <=== digest ===> |      - Valid Form B (tag is dropped)
+//	<=== REPOSITORY ===> : <=== TAG ======================> |      - Valid Form C
+//	<=== REPOSITORY ======================================> |    - Valid Form D
 //
 // Note: In the case of Valid Form B, TAG is dropped without any validation or
 // further consideration.
@@ -98,10 +98,12 @@ func ParseReference(artifact string) (Reference, error) {
 	}
 	registry, path := parts[0], parts[1]
 
+	var isTag bool
 	var repository string
 	var reference string
 	if index := strings.Index(path, "@"); index != -1 {
 		// `digest` found; Valid Form A (if not B)
+		isTag = false
 		repository = path[:index]
 		reference = path[index+1:]
 
@@ -112,6 +114,7 @@ func ParseReference(artifact string) (Reference, error) {
 		}
 	} else if index := strings.Index(path, ":"); index != -1 {
 		// `tag` found; Valid Form C
+		isTag = true
 		repository = path[:index]
 		reference = path[index+1:]
 	} else {
@@ -123,14 +126,14 @@ func ParseReference(artifact string) (Reference, error) {
 		Repository: repository,
 		Reference:  reference,
 	}
-	if err := res.Validate(); err != nil {
+	if err := res.Validate(isTag); err != nil {
 		return Reference{}, err
 	}
 	return res, nil
 }
 
 // Validate validates the entire reference.
-func (r Reference) Validate() error {
+func (r Reference) Validate(isTag bool) error {
 	err := r.ValidateRegistry()
 	if err != nil {
 		return err
@@ -139,7 +142,7 @@ func (r Reference) Validate() error {
 	if err != nil {
 		return err
 	}
-	return r.ValidateReference()
+	return r.ValidateReference(isTag)
 }
 
 // ValidateRegistry validates the registry.
@@ -160,16 +163,19 @@ func (r Reference) ValidateRepository() error {
 }
 
 // ValidateReference validates the reference.
-func (r Reference) ValidateReference() error {
-	if r.Reference == "" {
-		return nil
-	}
-	if _, err := r.Digest(); err == nil {
+func (r Reference) ValidateReference(isTag bool) error {
+	if r.Reference == "" || isTag && tagRegexp.MatchString(r.Reference) {
 		return nil
 	}
-	if !tagRegexp.MatchString(r.Reference) {
+
+	if isTag {
 		return fmt.Errorf("%w: invalid tag", errdef.ErrInvalidReference)
 	}
+
+	if _, err := r.Digest(); err != nil {
+		return fmt.Errorf("%w: invalid digest; %v", errdef.ErrInvalidReference, err)
+	}
+
 	return nil
 }
 

registry/remote/repository.go

@@ -196,17 +196,25 @@ func (r *Repository) FetchReference(ctx context.Context, reference string) (ocis
 func (r *Repository) ParseReference(reference string) (registry.Reference, error) {
 	ref, err := registry.ParseReference(reference)
 	if err != nil {
+		var isTag = true
+
 		// reference is not a FQDN
 		if index := strings.IndexByte(reference, '@'); index != -1 {
 			// drop tag since the digest is present
 			reference = reference[index+1:]
+			isTag = false
+		} else if index = strings.IndexByte(reference, ':'); index != -1 {
+			// no @ but :, so it can only be a digest
+			isTag = false
 		}
+
 		ref = registry.Reference{
 			Registry:   r.Reference.Registry,
 			Repository: r.Reference.Repository,
 			Reference:  reference,
 		}
-		if err = ref.ValidateReference(); err != nil {
+
+		if err = ref.ValidateReference(isTag); err != nil {
 			return registry.Reference{}, err
 		}
 	} else if ref.Registry != r.Reference.Registry || ref.Repository != r.Reference.Repository {

registry/remote/repository_test.go

@@ -3592,37 +3592,87 @@ func TestRepository_ParseReference(t *testing.T) {
 			wantErr: errdef.ErrInvalidReference,
 		},
 		{
-			name: "missing reference after @",
+			name: "registry mismatch",
 			repoRef: registry.Reference{
 				Registry:   "registry.example.com",
 				Repository: "hello-world",
 			},
 			args: args{
-				reference: "registry.example.com/hello-world@",
+				reference: "myregistry.example.com/hello-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9",
 			},
 			want:    registry.Reference{},
 			wantErr: errdef.ErrInvalidReference,
 		},
 		{
-			name: "registry mismatch",
+			name: "repository mismatch",
 			repoRef: registry.Reference{
 				Registry:   "registry.example.com",
 				Repository: "hello-world",
 			},
 			args: args{
-				reference: "myregistry.example.com/hello-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9",
+				reference: "registry.example.com/goodbye-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9",
 			},
 			want:    registry.Reference{},
 			wantErr: errdef.ErrInvalidReference,
 		},
 		{
-			name: "repository mismatch",
+			name: "digest posing as a tag",
 			repoRef: registry.Reference{
 				Registry:   "registry.example.com",
 				Repository: "hello-world",
 			},
 			args: args{
-				reference: "registry.example.com/goodbye-world:foobar@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9",
+				reference: "registry.example.com:5000/hello-world:sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+			},
+			want:    registry.Reference{},
+			wantErr: errdef.ErrInvalidReference,
+		},
+		{
+			name: "missing reference after the at sign",
+			repoRef: registry.Reference{
+				Registry:   "registry.example.com",
+				Repository: "hello-world",
+			},
+			args: args{
+				reference: "registry.example.com/hello-world@",
+			},
+			want:    registry.Reference{},
+			wantErr: errdef.ErrInvalidReference,
+		},
+		{
+			name: "missing reference after the colon",
+			repoRef: registry.Reference{
+				Registry: "localhost",
+			},
+			args: args{
+				reference: "localhost:5000/hello:",
+			},
+			want:    registry.Reference{},
+			wantErr: errdef.ErrInvalidReference,
+		},
+		{
+			name:    "zero-size tag, zero-size digest",
+			repoRef: registry.Reference{},
+			args: args{
+				reference: "localhost:5000/hello:@",
+			},
+			want:    registry.Reference{},
+			wantErr: errdef.ErrInvalidReference,
+		},
+		{
+			name:    "zero-size tag with valid digest",
+			repoRef: registry.Reference{},
+			args: args{
+				reference: "localhost:5000/hello:@sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+			},
+			want:    registry.Reference{},
+			wantErr: errdef.ErrInvalidReference,
+		},
+		{
+			name:    "valid tag with zero-size digest",
+			repoRef: registry.Reference{},
+			args: args{
+				reference: "localhost:5000/hello:foobar@",
 			},
 			want:    registry.Reference{},
 			wantErr: errdef.ErrInvalidReference,