fix(snapshot): validate object envelope sizes
- commit
- 5c945b24044c5fb37056b2894fa49dea025d216b
- author
- brice · · 2026-07-21 19:32 UTC
- parent
- dd9f60c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +85 -12
M internal/usecase/snapshot/manifest.go +16 -8
| @@ -12,11 +12,15 @@ import ( | |||
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | 14 | const ( | |
| 15 | - | manifestMagic = "VEVM" | |
| 16 | - | objectMagic = "VEVO" | |
| 17 | - | ManifestVersion = uint16(1) | |
| 18 | - | manifestHeaderSize = 16 | |
| 19 | - | maxObjectEnvelopeSize = maxSnapshotBytes + manifestHeaderSize + 5 | |
| 15 | + | manifestMagic = "VEVM" | |
| 16 | + | objectMagic = "VEVO" | |
| 17 | + | ManifestVersion = uint16(1) | |
| 18 | + | manifestHeaderSize = 16 | |
| 19 | + | objectEnvelopeBodyPrefixSize = 1 + 4 // object kind and payload length | |
| 20 | + | minObjectEnvelopeSize = manifestHeaderSize + objectEnvelopeBodyPrefixSize + 1 | |
| 21 | + | // maxDecodedBodySize includes the object kind and payload-length prefix. | |
| 22 | + | maxObjectEnvelopeSize = manifestHeaderSize + maxDecodedBodySize | |
| 23 | + | maxObjectPayloadSize = maxDecodedBodySize - objectEnvelopeBodyPrefixSize | |
| 20 | 24 | ) | |
| 21 | 25 | ||
| 22 | 26 | // SnapshotDigest is the shared content-address type used by the repository. | |
| @@ -144,7 +148,7 @@ func UnmarshalManifest(encoded []byte) (Manifest, error) { | |||
| 144 | 148 | // MarshalObject frames payload as VEVO and returns the bytes together with the | |
| 145 | 149 | // SHA-256 address of those complete bytes. | |
| 146 | 150 | func MarshalObject(kind ObjectKind, payload []byte) (ports.SnapshotObject, error) { | |
| 147 | - | if !validObjectKind(kind) || len(payload) == 0 || len(payload) > maxSnapshotBytes { | |
| 151 | + | if !validObjectKind(kind) || len(payload) == 0 || len(payload) > maxObjectPayloadSize { | |
| 148 | 152 | return ports.SnapshotObject{}, fmt.Errorf("%w: object", ErrInvalidData) | |
| 149 | 153 | } | |
| 150 | 154 | var w payloadWriter | |
| @@ -181,7 +185,7 @@ func PreflightObject(encoded []byte) (ObjectKind, []byte, error) { | |||
| 181 | 185 | return 0, nil, fmt.Errorf("%w: object kind", ErrInvalidData) | |
| 182 | 186 | } | |
| 183 | 187 | n := binary.BigEndian.Uint32(body[1:5]) | |
| 184 | - | if n == 0 || n > maxSnapshotBytes { | |
| 188 | + | if n == 0 || n > maxObjectPayloadSize { | |
| 185 | 189 | return 0, nil, fmt.Errorf("%w: object length", ErrInvalidData) | |
| 186 | 190 | } | |
| 187 | 191 | if uint64(n) > uint64(len(body)-5) { | |
| @@ -424,11 +428,15 @@ func validatePaneRefs(p ManifestPane) error { | |||
| 424 | 428 | return validateObjectRef(p.Visible, Visible) | |
| 425 | 429 | } | |
| 426 | 430 | func validateObjectRef(ref ObjectRef, want ObjectKind) error { | |
| 427 | - | if ref.Kind != want || ref.Size < manifestHeaderSize+5 || ref.Size > maxObjectEnvelopeSize || isZeroDigest(ref.Digest) { | |
| 431 | + | if ref.Kind != want || !validObjectEnvelopeSize(ref.Size) || isZeroDigest(ref.Digest) { | |
| 428 | 432 | return fmt.Errorf("%w: object reference", ErrInvalidData) | |
| 429 | 433 | } | |
| 430 | 434 | return nil | |
| 431 | 435 | } | |
| 436 | + | func validObjectEnvelopeSize(size uint32) bool { | |
| 437 | + | return size >= minObjectEnvelopeSize && size <= maxObjectEnvelopeSize | |
| 438 | + | } | |
| 439 | + | ||
| 432 | 440 | func validObjectKind(kind ObjectKind) bool { | |
| 433 | 441 | return kind == HistoryChunk || kind == HistoryTail || kind == Visible | |
| 434 | 442 | } | |
| @@ -12,11 +12,15 @@ import ( | |||||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | const ( | 14 | const ( | ||
| 15 | - | manifestMagic = "VEVM" | 15 | + | manifestMagic = "VEVM" |
| 16 | - | objectMagic = "VEVO" | 16 | + | objectMagic = "VEVO" |
| 17 | - | ManifestVersion = uint16(1) | 17 | + | ManifestVersion = uint16(1) |
| 18 | - | manifestHeaderSize = 16 | 18 | + | manifestHeaderSize = 16 |
| 19 | - | maxObjectEnvelopeSize = maxSnapshotBytes + manifestHeaderSize + 5 | 19 | + | objectEnvelopeBodyPrefixSize = 1 + 4 // object kind and payload length |
| 20 | + | minObjectEnvelopeSize = manifestHeaderSize + objectEnvelopeBodyPrefixSize + 1 | |||
| 21 | + | // maxDecodedBodySize includes the object kind and payload-length prefix. | |||
| 22 | + | maxObjectEnvelopeSize = manifestHeaderSize + maxDecodedBodySize | |||
| 23 | + | maxObjectPayloadSize = maxDecodedBodySize - objectEnvelopeBodyPrefixSize | |||
| 20 | ) | 24 | ) | ||
| 21 | 25 | ||||
| 22 | // SnapshotDigest is the shared content-address type used by the repository. | 26 | // SnapshotDigest is the shared content-address type used by the repository. | ||
| @@ -144,7 +148,7 @@ func UnmarshalManifest(encoded []byte) (Manifest, error) { | |||||
| 144 | // MarshalObject frames payload as VEVO and returns the bytes together with the | 148 | // MarshalObject frames payload as VEVO and returns the bytes together with the | ||
| 145 | // SHA-256 address of those complete bytes. | 149 | // SHA-256 address of those complete bytes. | ||
| 146 | func MarshalObject(kind ObjectKind, payload []byte) (ports.SnapshotObject, error) { | 150 | func MarshalObject(kind ObjectKind, payload []byte) (ports.SnapshotObject, error) { | ||
| 147 | - | if !validObjectKind(kind) || len(payload) == 0 || len(payload) > maxSnapshotBytes { | 151 | + | if !validObjectKind(kind) || len(payload) == 0 || len(payload) > maxObjectPayloadSize { |
| 148 | return ports.SnapshotObject{}, fmt.Errorf("%w: object", ErrInvalidData) | 152 | return ports.SnapshotObject{}, fmt.Errorf("%w: object", ErrInvalidData) | ||
| 149 | } | 153 | } | ||
| 150 | var w payloadWriter | 154 | var w payloadWriter | ||
| @@ -181,7 +185,7 @@ func PreflightObject(encoded []byte) (ObjectKind, []byte, error) { | |||||
| 181 | return 0, nil, fmt.Errorf("%w: object kind", ErrInvalidData) | 185 | return 0, nil, fmt.Errorf("%w: object kind", ErrInvalidData) | ||
| 182 | } | 186 | } | ||
| 183 | n := binary.BigEndian.Uint32(body[1:5]) | 187 | n := binary.BigEndian.Uint32(body[1:5]) | ||
| 184 | - | if n == 0 || n > maxSnapshotBytes { | 188 | + | if n == 0 || n > maxObjectPayloadSize { |
| 185 | return 0, nil, fmt.Errorf("%w: object length", ErrInvalidData) | 189 | return 0, nil, fmt.Errorf("%w: object length", ErrInvalidData) | ||
| 186 | } | 190 | } | ||
| 187 | if uint64(n) > uint64(len(body)-5) { | 191 | if uint64(n) > uint64(len(body)-5) { | ||
| @@ -424,11 +428,15 @@ func validatePaneRefs(p ManifestPane) error { | |||||
| 424 | return validateObjectRef(p.Visible, Visible) | 428 | return validateObjectRef(p.Visible, Visible) | ||
| 425 | } | 429 | } | ||
| 426 | func validateObjectRef(ref ObjectRef, want ObjectKind) error { | 430 | func validateObjectRef(ref ObjectRef, want ObjectKind) error { | ||
| 427 | - | if ref.Kind != want || ref.Size < manifestHeaderSize+5 || ref.Size > maxObjectEnvelopeSize || isZeroDigest(ref.Digest) { | 431 | + | if ref.Kind != want || !validObjectEnvelopeSize(ref.Size) || isZeroDigest(ref.Digest) { |
| 428 | return fmt.Errorf("%w: object reference", ErrInvalidData) | 432 | return fmt.Errorf("%w: object reference", ErrInvalidData) | ||
| 429 | } | 433 | } | ||
| 430 | return nil | 434 | return nil | ||
| 431 | } | 435 | } | ||
| 436 | + | func validObjectEnvelopeSize(size uint32) bool { | |||
| 437 | + | return size >= minObjectEnvelopeSize && size <= maxObjectEnvelopeSize | |||
| 438 | + | } | |||
| 439 | + | ||||
| 432 | func validObjectKind(kind ObjectKind) bool { | 440 | func validObjectKind(kind ObjectKind) bool { | ||
| 433 | return kind == HistoryChunk || kind == HistoryTail || kind == Visible | 441 | return kind == HistoryChunk || kind == HistoryTail || kind == Visible | ||
| 434 | } | 442 | } | ||
M internal/usecase/snapshot/manifest_preflight.go +1 -1
| @@ -242,7 +242,7 @@ func preflightObjectRef(r *payloadReader, want ObjectKind) error { | |||
| 242 | 242 | if err != nil { | |
| 243 | 243 | return err | |
| 244 | 244 | } | |
| 245 | - | if size < manifestHeaderSize+5 || size > maxObjectEnvelopeSize { | |
| 245 | + | if !validObjectEnvelopeSize(size) { | |
| 246 | 246 | return fmt.Errorf("%w: object size", ErrInvalidData) | |
| 247 | 247 | } | |
| 248 | 248 | return nil | |
| @@ -242,7 +242,7 @@ func preflightObjectRef(r *payloadReader, want ObjectKind) error { | |||||
| 242 | if err != nil { | 242 | if err != nil { | ||
| 243 | return err | 243 | return err | ||
| 244 | } | 244 | } | ||
| 245 | - | if size < manifestHeaderSize+5 || size > maxObjectEnvelopeSize { | 245 | + | if !validObjectEnvelopeSize(size) { |
| 246 | return fmt.Errorf("%w: object size", ErrInvalidData) | 246 | return fmt.Errorf("%w: object size", ErrInvalidData) | ||
| 247 | } | 247 | } | ||
| 248 | return nil | 248 | return nil | ||
M internal/usecase/snapshot/manifest_test.go +68 -3
| @@ -85,8 +85,8 @@ func TestObjectCodecReturnsContentAddressedObjectAndPayload(t *testing.T) { | |||
| 85 | 85 | } | |
| 86 | 86 | ||
| 87 | 87 | func TestManifestDimensionBoundaries(t *testing.T) { | |
| 88 | - | ref := ObjectRef{Kind: HistoryTail, Digest: ports.SnapshotDigest{1}, Size: manifestHeaderSize + 5} | |
| 89 | - | visible := ObjectRef{Kind: Visible, Digest: ports.SnapshotDigest{2}, Size: manifestHeaderSize + 5} | |
| 88 | + | ref := ObjectRef{Kind: HistoryTail, Digest: ports.SnapshotDigest{1}, Size: minObjectEnvelopeSize} | |
| 89 | + | visible := ObjectRef{Kind: Visible, Digest: ports.SnapshotDigest{2}, Size: minObjectEnvelopeSize} | |
| 90 | 90 | for _, tc := range []struct { | |
| 91 | 91 | name string | |
| 92 | 92 | cols, rows uint16 | |
| @@ -110,9 +110,74 @@ func TestManifestRejectsImpossibleObjectEnvelopeSize(t *testing.T) { | |||
| 110 | 110 | manifest := Manifest{Name: "named", Tabs: []ManifestTab{{Cols: 1, Rows: 1, Panes: []ManifestPane{{ | |
| 111 | 111 | ID: "pane", | |
| 112 | 112 | Tail: ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: 1}, | |
| 113 | - | Visible: ObjectRef{Kind: Visible, Digest: SnapshotDigest{2}, Size: manifestHeaderSize + 5}, | |
| 113 | + | Visible: ObjectRef{Kind: Visible, Digest: SnapshotDigest{2}, Size: minObjectEnvelopeSize}, | |
| 114 | 114 | }}}}} | |
| 115 | 115 | if _, err := MarshalManifest(manifest); !errors.Is(err, ErrInvalidData) { | |
| 116 | 116 | t.Fatalf("MarshalManifest() error = %v, want invalid data", err) | |
| 117 | 117 | } | |
| 118 | 118 | } | |
| 119 | + | ||
| 120 | + | func TestObjectRefEnvelopeSizeBoundaries(t *testing.T) { | |
| 121 | + | const ( | |
| 122 | + | minimum = manifestHeaderSize + 1 + 4 + 1 | |
| 123 | + | maximum = manifestHeaderSize + maxDecodedBodySize | |
| 124 | + | ) | |
| 125 | + | if minObjectEnvelopeSize != minimum || maxObjectEnvelopeSize != maximum { | |
| 126 | + | t.Fatalf("object envelope range = [%d, %d], want [%d, %d]", minObjectEnvelopeSize, maxObjectEnvelopeSize, minimum, maximum) | |
| 127 | + | } | |
| 128 | + | object, err := MarshalObject(HistoryTail, []byte{0}) | |
| 129 | + | if err != nil { | |
| 130 | + | t.Fatalf("MarshalObject() error = %v", err) | |
| 131 | + | } | |
| 132 | + | if len(object.Data) != minimum { | |
| 133 | + | t.Fatalf("MarshalObject() size = %d, want %d", len(object.Data), minimum) | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | for _, tc := range []struct { | |
| 137 | + | name string | |
| 138 | + | size uint32 | |
| 139 | + | valid bool | |
| 140 | + | }{ | |
| 141 | + | {name: "one below minimum", size: minimum - 1}, | |
| 142 | + | {name: "minimum", size: minimum, valid: true}, | |
| 143 | + | {name: "maximum declaration", size: maximum, valid: true}, | |
| 144 | + | {name: "one above maximum", size: maximum + 1}, | |
| 145 | + | } { | |
| 146 | + | t.Run(tc.name, func(t *testing.T) { | |
| 147 | + | manifest := manifestWithTailSize(tc.size) | |
| 148 | + | encoded, err := MarshalManifest(manifest) | |
| 149 | + | if tc.valid { | |
| 150 | + | if err != nil { | |
| 151 | + | t.Fatalf("MarshalManifest() error = %v", err) | |
| 152 | + | } | |
| 153 | + | if _, err := UnmarshalManifest(encoded); err != nil { | |
| 154 | + | t.Fatalf("UnmarshalManifest() error = %v", err) | |
| 155 | + | } | |
| 156 | + | } else if !errors.Is(err, ErrInvalidData) { | |
| 157 | + | t.Fatalf("MarshalManifest() error = %v, want invalid data", err) | |
| 158 | + | } | |
| 159 | + | ||
| 160 | + | var w payloadWriter | |
| 161 | + | writeObjectRef(&w, ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: tc.size}) | |
| 162 | + | err = preflightObjectRef(&payloadReader{b: w.b}, HistoryTail) | |
| 163 | + | if tc.valid && err != nil { | |
| 164 | + | t.Fatalf("preflightObjectRef() error = %v", err) | |
| 165 | + | } | |
| 166 | + | if !tc.valid && !errors.Is(err, ErrInvalidData) { | |
| 167 | + | t.Fatalf("preflightObjectRef() error = %v, want invalid data", err) | |
| 168 | + | } | |
| 169 | + | }) | |
| 170 | + | } | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | func manifestWithTailSize(size uint32) Manifest { | |
| 174 | + | return Manifest{Name: "named", Tabs: []ManifestTab{{ | |
| 175 | + | Cols: 1, | |
| 176 | + | Rows: 1, | |
| 177 | + | Panes: []ManifestPane{{ | |
| 178 | + | ID: "pane", | |
| 179 | + | Tail: ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: size}, | |
| 180 | + | Visible: ObjectRef{Kind: Visible, Digest: SnapshotDigest{2}, Size: minObjectEnvelopeSize}, | |
| 181 | + | }}, | |
| 182 | + | }}} | |
| 183 | + | } | |
| @@ -85,8 +85,8 @@ func TestObjectCodecReturnsContentAddressedObjectAndPayload(t *testing.T) { | |||||
| 85 | } | 85 | } | ||
| 86 | 86 | ||||
| 87 | func TestManifestDimensionBoundaries(t *testing.T) { | 87 | func TestManifestDimensionBoundaries(t *testing.T) { | ||
| 88 | - | ref := ObjectRef{Kind: HistoryTail, Digest: ports.SnapshotDigest{1}, Size: manifestHeaderSize + 5} | 88 | + | ref := ObjectRef{Kind: HistoryTail, Digest: ports.SnapshotDigest{1}, Size: minObjectEnvelopeSize} |
| 89 | - | visible := ObjectRef{Kind: Visible, Digest: ports.SnapshotDigest{2}, Size: manifestHeaderSize + 5} | 89 | + | visible := ObjectRef{Kind: Visible, Digest: ports.SnapshotDigest{2}, Size: minObjectEnvelopeSize} |
| 90 | for _, tc := range []struct { | 90 | for _, tc := range []struct { | ||
| 91 | name string | 91 | name string | ||
| 92 | cols, rows uint16 | 92 | cols, rows uint16 | ||
| @@ -110,9 +110,74 @@ func TestManifestRejectsImpossibleObjectEnvelopeSize(t *testing.T) { | |||||
| 110 | manifest := Manifest{Name: "named", Tabs: []ManifestTab{{Cols: 1, Rows: 1, Panes: []ManifestPane{{ | 110 | manifest := Manifest{Name: "named", Tabs: []ManifestTab{{Cols: 1, Rows: 1, Panes: []ManifestPane{{ | ||
| 111 | ID: "pane", | 111 | ID: "pane", | ||
| 112 | Tail: ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: 1}, | 112 | Tail: ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: 1}, | ||
| 113 | - | Visible: ObjectRef{Kind: Visible, Digest: SnapshotDigest{2}, Size: manifestHeaderSize + 5}, | 113 | + | Visible: ObjectRef{Kind: Visible, Digest: SnapshotDigest{2}, Size: minObjectEnvelopeSize}, |
| 114 | }}}}} | 114 | }}}}} | ||
| 115 | if _, err := MarshalManifest(manifest); !errors.Is(err, ErrInvalidData) { | 115 | if _, err := MarshalManifest(manifest); !errors.Is(err, ErrInvalidData) { | ||
| 116 | t.Fatalf("MarshalManifest() error = %v, want invalid data", err) | 116 | t.Fatalf("MarshalManifest() error = %v, want invalid data", err) | ||
| 117 | } | 117 | } | ||
| 118 | } | 118 | } | ||
| 119 | + | ||||
| 120 | + | func TestObjectRefEnvelopeSizeBoundaries(t *testing.T) { | |||
| 121 | + | const ( | |||
| 122 | + | minimum = manifestHeaderSize + 1 + 4 + 1 | |||
| 123 | + | maximum = manifestHeaderSize + maxDecodedBodySize | |||
| 124 | + | ) | |||
| 125 | + | if minObjectEnvelopeSize != minimum || maxObjectEnvelopeSize != maximum { | |||
| 126 | + | t.Fatalf("object envelope range = [%d, %d], want [%d, %d]", minObjectEnvelopeSize, maxObjectEnvelopeSize, minimum, maximum) | |||
| 127 | + | } | |||
| 128 | + | object, err := MarshalObject(HistoryTail, []byte{0}) | |||
| 129 | + | if err != nil { | |||
| 130 | + | t.Fatalf("MarshalObject() error = %v", err) | |||
| 131 | + | } | |||
| 132 | + | if len(object.Data) != minimum { | |||
| 133 | + | t.Fatalf("MarshalObject() size = %d, want %d", len(object.Data), minimum) | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | for _, tc := range []struct { | |||
| 137 | + | name string | |||
| 138 | + | size uint32 | |||
| 139 | + | valid bool | |||
| 140 | + | }{ | |||
| 141 | + | {name: "one below minimum", size: minimum - 1}, | |||
| 142 | + | {name: "minimum", size: minimum, valid: true}, | |||
| 143 | + | {name: "maximum declaration", size: maximum, valid: true}, | |||
| 144 | + | {name: "one above maximum", size: maximum + 1}, | |||
| 145 | + | } { | |||
| 146 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 147 | + | manifest := manifestWithTailSize(tc.size) | |||
| 148 | + | encoded, err := MarshalManifest(manifest) | |||
| 149 | + | if tc.valid { | |||
| 150 | + | if err != nil { | |||
| 151 | + | t.Fatalf("MarshalManifest() error = %v", err) | |||
| 152 | + | } | |||
| 153 | + | if _, err := UnmarshalManifest(encoded); err != nil { | |||
| 154 | + | t.Fatalf("UnmarshalManifest() error = %v", err) | |||
| 155 | + | } | |||
| 156 | + | } else if !errors.Is(err, ErrInvalidData) { | |||
| 157 | + | t.Fatalf("MarshalManifest() error = %v, want invalid data", err) | |||
| 158 | + | } | |||
| 159 | + | ||||
| 160 | + | var w payloadWriter | |||
| 161 | + | writeObjectRef(&w, ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: tc.size}) | |||
| 162 | + | err = preflightObjectRef(&payloadReader{b: w.b}, HistoryTail) | |||
| 163 | + | if tc.valid && err != nil { | |||
| 164 | + | t.Fatalf("preflightObjectRef() error = %v", err) | |||
| 165 | + | } | |||
| 166 | + | if !tc.valid && !errors.Is(err, ErrInvalidData) { | |||
| 167 | + | t.Fatalf("preflightObjectRef() error = %v, want invalid data", err) | |||
| 168 | + | } | |||
| 169 | + | }) | |||
| 170 | + | } | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | func manifestWithTailSize(size uint32) Manifest { | |||
| 174 | + | return Manifest{Name: "named", Tabs: []ManifestTab{{ | |||
| 175 | + | Cols: 1, | |||
| 176 | + | Rows: 1, | |||
| 177 | + | Panes: []ManifestPane{{ | |||
| 178 | + | ID: "pane", | |||
| 179 | + | Tail: ObjectRef{Kind: HistoryTail, Digest: SnapshotDigest{1}, Size: size}, | |||
| 180 | + | Visible: ObjectRef{Kind: Visible, Digest: SnapshotDigest{2}, Size: minObjectEnvelopeSize}, | |||
| 181 | + | }}, | |||
| 182 | + | }}} | |||
| 183 | + | } | |||