Skip to content

Commit 283d392

Browse files
WwwsylviaWwwsylvia
authored
fix: fix panics caused by bad digests (#926)
1. Always validate the digest before calling functions like `desc.Digest.Algorithm()` and `desc.Digest.Verifier()`, which panics when encountering invalid digest 2. Refactor `buildReferrersTag` to return an error 3. Add corresponding unit tests Before this PR, pushing descriptors with bad digest to memory store and file store would panic. After this PR, pushing descriptors with bad digest to memory store and file store results in an error. Fixes: #923 Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
1 parent d51a392 commit 283d392

File tree

9 files changed

+558
-11
lines changed

9 files changed

+558
-11
lines changed

‎content/file/file_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package file
1818
import (
1919
"bytes"
2020
"context"
21+
"crypto/sha1"
2122
_ "crypto/sha256"
2223
"encoding/json"
2324
"errors"
@@ -3354,6 +3355,81 @@ func TestStore_resolveWritePath_Overwrite(t *testing.T) {
33543355
})
33553356
}
33563357

3358+
func TestStore_BadDigest(t *testing.T) {
3359+
data := []byte("hello world")
3360+
ref := "foobar"
3361+
3362+
t.Run("invalid digest", func(t *testing.T) {
3363+
desc := ocispec.Descriptor{
3364+
MediaType: "application/test",
3365+
Size: int64(len(data)),
3366+
Digest: "invalid-digest",
3367+
}
3368+
3369+
s, err := New(t.TempDir())
3370+
if err != nil {
3371+
t.Fatal("Store.New() error =", err)
3372+
}
3373+
ctx := context.Background()
3374+
if err := s.Push(ctx, desc, bytes.NewReader(data)); !errors.Is(err, digest.ErrDigestInvalidFormat) {
3375+
t.Errorf("Store.Push() error = %v, wantErr %v", err, digest.ErrDigestInvalidFormat)
3376+
3377+
}
3378+
3379+
if err := s.Tag(ctx, desc, ref); !errors.Is(err, errdef.ErrNotFound) {
3380+
t.Errorf("Store.Tag() error = %v, wantErr %v", err, errdef.ErrNotFound)
3381+
}
3382+
3383+
if _, err := s.Exists(ctx, desc); err != nil {
3384+
t.Errorf("Store.Exists() error = %v, wantErr %v", err, nil)
3385+
}
3386+
3387+
if _, err := s.Fetch(ctx, desc); !errors.Is(err, errdef.ErrNotFound) {
3388+
t.Errorf("Store.Fetch() error = %v, wantErr %v", err, errdef.ErrNotFound)
3389+
}
3390+
3391+
if _, err := s.Predecessors(ctx, desc); err != nil {
3392+
t.Errorf("Store.Predecessors() error = %v, wantErr %v", err, nil)
3393+
}
3394+
})
3395+
3396+
t.Run("unsupported digest (sha1)", func(t *testing.T) {
3397+
h := sha1.New()
3398+
h.Write(data)
3399+
desc := ocispec.Descriptor{
3400+
MediaType: "application/test",
3401+
Size: int64(len(data)),
3402+
Digest: digest.NewDigestFromBytes("sha1", h.Sum(nil)),
3403+
}
3404+
3405+
s, err := New(t.TempDir())
3406+
if err != nil {
3407+
t.Fatal("Store.New() error =", err)
3408+
}
3409+
ctx := context.Background()
3410+
if err := s.Push(ctx, desc, bytes.NewReader(data)); !errors.Is(err, digest.ErrDigestUnsupported) {
3411+
t.Errorf("Store.Push() error = %v, wantErr %v", err, digest.ErrDigestUnsupported)
3412+
3413+
}
3414+
3415+
if err := s.Tag(ctx, desc, ref); !errors.Is(err, errdef.ErrNotFound) {
3416+
t.Errorf("Store.Tag() error = %v, wantErr %v", err, errdef.ErrNotFound)
3417+
}
3418+
3419+
if _, err := s.Exists(ctx, desc); err != nil {
3420+
t.Errorf("Store.Exists() error = %v, wantErr %v", err, nil)
3421+
}
3422+
3423+
if _, err := s.Fetch(ctx, desc); !errors.Is(err, errdef.ErrNotFound) {
3424+
t.Errorf("Store.Fetch() error = %v, wantErr %v", err, errdef.ErrNotFound)
3425+
}
3426+
3427+
if _, err := s.Predecessors(ctx, desc); err != nil {
3428+
t.Errorf("Store.Predecessors() error = %v, wantErr %v", err, nil)
3429+
}
3430+
})
3431+
}
3432+
33573433
func equalDescriptorSet(actual []ocispec.Descriptor, expected []ocispec.Descriptor) bool {
33583434
if len(actual) != len(expected) {
33593435
return false

‎content/memory/memory_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package memory
1818
import (
1919
"bytes"
2020
"context"
21+
"crypto/sha1"
2122
_ "crypto/sha256"
2223
"encoding/json"
2324
"errors"
@@ -402,6 +403,75 @@ func TestStorePredecessors(t *testing.T) {
402403
}
403404
}
404405

406+
func TestStore_BadDigest(t *testing.T) {
407+
data := []byte("hello world")
408+
ref := "foobar"
409+
410+
t.Run("invalid digest", func(t *testing.T) {
411+
desc := ocispec.Descriptor{
412+
MediaType: "application/test",
413+
Size: int64(len(data)),
414+
Digest: "invalid-digest",
415+
}
416+
417+
s := New()
418+
ctx := context.Background()
419+
if err := s.Push(ctx, desc, bytes.NewReader(data)); !errors.Is(err, digest.ErrDigestInvalidFormat) {
420+
t.Errorf("Store.Push() error = %v, wantErr %v", err, digest.ErrDigestInvalidFormat)
421+
422+
}
423+
424+
if err := s.Tag(ctx, desc, ref); !errors.Is(err, errdef.ErrNotFound) {
425+
t.Errorf("Store.Tag() error = %v, wantErr %v", err, errdef.ErrNotFound)
426+
}
427+
428+
if _, err := s.Exists(ctx, desc); err != nil {
429+
t.Errorf("Store.Exists() error = %v, wantErr %v", err, nil)
430+
}
431+
432+
if _, err := s.Fetch(ctx, desc); !errors.Is(err, errdef.ErrNotFound) {
433+
t.Errorf("Store.Fetch() error = %v, wantErr %v", err, errdef.ErrNotFound)
434+
}
435+
436+
if _, err := s.Predecessors(ctx, desc); err != nil {
437+
t.Errorf("Store.Predecessors() error = %v, wantErr %v", err, nil)
438+
}
439+
})
440+
441+
t.Run("unsupported digest (sha1)", func(t *testing.T) {
442+
h := sha1.New()
443+
h.Write(data)
444+
desc := ocispec.Descriptor{
445+
MediaType: "application/test",
446+
Size: int64(len(data)),
447+
Digest: digest.NewDigestFromBytes("sha1", h.Sum(nil)),
448+
}
449+
450+
s := New()
451+
ctx := context.Background()
452+
if err := s.Push(ctx, desc, bytes.NewReader(data)); !errors.Is(err, digest.ErrDigestUnsupported) {
453+
t.Errorf("Store.Push() error = %v, wantErr %v", err, digest.ErrDigestUnsupported)
454+
455+
}
456+
457+
if err := s.Tag(ctx, desc, ref); !errors.Is(err, errdef.ErrNotFound) {
458+
t.Errorf("Store.Tag() error = %v, wantErr %v", err, errdef.ErrNotFound)
459+
}
460+
461+
if _, err := s.Exists(ctx, desc); err != nil {
462+
t.Errorf("Store.Exists() error = %v, wantErr %v", err, nil)
463+
}
464+
465+
if _, err := s.Fetch(ctx, desc); !errors.Is(err, errdef.ErrNotFound) {
466+
t.Errorf("Store.Fetch() error = %v, wantErr %v", err, errdef.ErrNotFound)
467+
}
468+
469+
if _, err := s.Predecessors(ctx, desc); err != nil {
470+
t.Errorf("Store.Predecessors() error = %v, wantErr %v", err, nil)
471+
}
472+
})
473+
}
474+
405475
func equalDescriptorSet(actual []ocispec.Descriptor, expected []ocispec.Descriptor) bool {
406476
if len(actual) != len(expected) {
407477
return false

‎content/oci/oci_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package oci
1818
import (
1919
"bytes"
2020
"context"
21+
"crypto/sha1"
2122
_ "crypto/sha256"
2223
"encoding/json"
2324
"errors"
@@ -2871,3 +2872,78 @@ func Test_isContextDone(t *testing.T) {
28712872
t.Errorf("expect error = %v, got %v", context.Canceled, err)
28722873
}
28732874
}
2875+
2876+
func TestStore_BadDigest(t *testing.T) {
2877+
data := []byte("hello world")
2878+
ref := "foobar"
2879+
2880+
t.Run("invalid digest", func(t *testing.T) {
2881+
desc := ocispec.Descriptor{
2882+
MediaType: "application/test",
2883+
Size: int64(len(data)),
2884+
Digest: "invalid-digest",
2885+
}
2886+
2887+
s, err := New(t.TempDir())
2888+
if err != nil {
2889+
t.Fatal("Store.New() error =", err)
2890+
}
2891+
ctx := context.Background()
2892+
if err := s.Push(ctx, desc, bytes.NewReader(data)); !errors.Is(err, errdef.ErrInvalidDigest) {
2893+
t.Errorf("Store.Push() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2894+
2895+
}
2896+
2897+
if err := s.Tag(ctx, desc, ref); !errors.Is(err, errdef.ErrInvalidDigest) {
2898+
t.Errorf("Store.Tag() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2899+
}
2900+
2901+
if _, err := s.Exists(ctx, desc); !errors.Is(err, errdef.ErrInvalidDigest) {
2902+
t.Errorf("Store.Exists() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2903+
}
2904+
2905+
if _, err := s.Fetch(ctx, desc); !errors.Is(err, errdef.ErrInvalidDigest) {
2906+
t.Errorf("Store.Fetch() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2907+
}
2908+
2909+
if _, err := s.Predecessors(ctx, desc); err != nil {
2910+
t.Errorf("Store.Predecessors() error = %v, wantErr %v", err, nil)
2911+
}
2912+
})
2913+
2914+
t.Run("unsupported digest (sha1)", func(t *testing.T) {
2915+
h := sha1.New()
2916+
h.Write(data)
2917+
desc := ocispec.Descriptor{
2918+
MediaType: "application/test",
2919+
Size: int64(len(data)),
2920+
Digest: digest.NewDigestFromBytes("sha1", h.Sum(nil)),
2921+
}
2922+
2923+
s, err := New(t.TempDir())
2924+
if err != nil {
2925+
t.Fatal("Store.New() error =", err)
2926+
}
2927+
ctx := context.Background()
2928+
if err := s.Push(ctx, desc, bytes.NewReader(data)); !errors.Is(err, errdef.ErrInvalidDigest) {
2929+
t.Errorf("Store.Push() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2930+
2931+
}
2932+
2933+
if err := s.Tag(ctx, desc, ref); !errors.Is(err, errdef.ErrInvalidDigest) {
2934+
t.Errorf("Store.Tag() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2935+
}
2936+
2937+
if _, err := s.Exists(ctx, desc); !errors.Is(err, errdef.ErrInvalidDigest) {
2938+
t.Errorf("Store.Exists() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2939+
}
2940+
2941+
if _, err := s.Fetch(ctx, desc); !errors.Is(err, errdef.ErrInvalidDigest) {
2942+
t.Errorf("Store.Fetch() error = %v, wantErr %v", err, errdef.ErrInvalidDigest)
2943+
}
2944+
2945+
if _, err := s.Predecessors(ctx, desc); err != nil {
2946+
t.Errorf("Store.Predecessors() error = %v, wantErr %v", err, nil)
2947+
}
2948+
})
2949+
}

‎content/reader.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,11 @@ func (vr *VerifyReader) Verify() error {
9999

100100
// NewVerifyReader wraps r for reading content with verification against desc.
101101
func NewVerifyReader(r io.Reader, desc ocispec.Descriptor) *VerifyReader {
102+
if err := desc.Digest.Validate(); err != nil {
103+
return &VerifyReader{
104+
err: fmt.Errorf("failed to validate %s: %w", desc.Digest, err),
105+
}
106+
}
102107
verifier := desc.Digest.Verifier()
103108
lr := &io.LimitedReader{
104109
R: io.TeeReader(r, verifier),

‎content/reader_test.go

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package content
1717

1818
import (
1919
"bytes"
20+
"crypto/sha1"
2021
_ "crypto/sha256"
2122
"errors"
2223
"io"
@@ -64,6 +65,9 @@ func TestVerifyReader_Read(t *testing.T) {
6465
// mismatched content and descriptor with sufficient buffer
6566
r = bytes.NewReader([]byte("bar"))
6667
vr = NewVerifyReader(r, desc)
68+
if err != nil {
69+
t.Fatal("NewVerifyReaderSafe() error = ", err)
70+
}
6771
buf = make([]byte, 5)
6872
n, err = vr.Read(buf)
6973
if err != nil {
@@ -97,7 +101,8 @@ func TestVerifyReader_Verify(t *testing.T) {
97101
desc = ocispec.Descriptor{
98102
MediaType: ocispec.MediaTypeImageLayer,
99103
Digest: digest.FromBytes(content),
100-
Size: int64(len(content)) - 1}
104+
Size: int64(len(content)) - 1,
105+
}
101106
vr = NewVerifyReader(r, desc)
102107
buf = make([]byte, len(content))
103108
if _, err := vr.Read(buf); err != nil {
@@ -117,7 +122,8 @@ func TestVerifyReader_Verify(t *testing.T) {
117122
desc = ocispec.Descriptor{
118123
MediaType: ocispec.MediaTypeImageLayer,
119124
Digest: digest.FromBytes(content),
120-
Size: int64(len(content)) + 1}
125+
Size: int64(len(content)) + 1,
126+
}
121127
vr = NewVerifyReader(r, desc)
122128
buf = make([]byte, len(content))
123129
if _, err := vr.Read(buf); err != nil {
@@ -188,7 +194,7 @@ func TestReadAll_ReadSizeLargerThanDescriptorSize(t *testing.T) {
188194
}
189195
}
190196

191-
func TestReadAll_InvalidDigest(t *testing.T) {
197+
func TestReadAll_MismatchedDigest(t *testing.T) {
192198
content := []byte("example content")
193199
desc := NewDescriptorFromBytes("test", []byte("another content"))
194200
r := bytes.NewReader([]byte(content))
@@ -198,6 +204,36 @@ func TestReadAll_InvalidDigest(t *testing.T) {
198204
}
199205
}
200206

207+
func TestReadAll_InvalidDigest(t *testing.T) {
208+
content := []byte("example content")
209+
desc := ocispec.Descriptor{
210+
MediaType: ocispec.MediaTypeImageLayer,
211+
Digest: "invalid-digest",
212+
Size: int64(len(content)),
213+
}
214+
r := bytes.NewReader([]byte(content))
215+
_, err := ReadAll(r, desc)
216+
if wantErr := digest.ErrDigestInvalidFormat; !errors.Is(err, wantErr) {
217+
t.Errorf("ReadAll() error = %v, want %v", err, wantErr)
218+
}
219+
}
220+
221+
func TestReadAll_UnsupportedAlgorithm_SHA1(t *testing.T) {
222+
content := []byte("example content")
223+
h := sha1.New()
224+
h.Write(content)
225+
desc := ocispec.Descriptor{
226+
MediaType: ocispec.MediaTypeImageLayer,
227+
Digest: digest.NewDigestFromBytes("sha1", h.Sum(nil)),
228+
Size: int64(len(content)),
229+
}
230+
r := bytes.NewReader([]byte(content))
231+
_, err := ReadAll(r, desc)
232+
if wantErr := digest.ErrDigestUnsupported; !errors.Is(err, wantErr) {
233+
t.Errorf("ReadAll() error = %v, want %v", err, wantErr)
234+
}
235+
}
236+
201237
func TestReadAll_EmptyContent(t *testing.T) {
202238
content := []byte("")
203239
desc := NewDescriptorFromBytes("test", content)

‎registry/remote/referrers.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package remote
1717

1818
import (
1919
"errors"
20+
"fmt"
2021
"strings"
2122

2223
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -103,10 +104,13 @@ func (e *ReferrersError) IsReferrersIndexDelete() bool {
103104
// buildReferrersTag builds the referrers tag for the given manifest descriptor.
104105
// Format: <algorithm>-<digest>
105106
// Reference: https://github.com/opencontainers/distribution-spec/blob/v1.1.1/spec.md#unavailable-referrers-api
106-
func buildReferrersTag(desc ocispec.Descriptor) string {
107+
func buildReferrersTag(desc ocispec.Descriptor) (string, error) {
108+
if err := desc.Digest.Validate(); err != nil {
109+
return "", fmt.Errorf("failed to build referrers tag for %s: %w", desc.Digest, err)
110+
}
107111
alg := desc.Digest.Algorithm().String()
108112
encoded := desc.Digest.Encoded()
109-
return alg + "-" + encoded
113+
return alg + "-" + encoded, nil
110114
}
111115

112116
// isReferrersFilterApplied checks if requsted is in the applied filter list.

0 commit comments

Comments
 (0)