Skip to content

Commit e50c2d7

Browse files
codysoylandcodysoyland
authored
Add additional validation for nil elements in Bundles (#285)
Signed-off-by: Cody Soyland <[email protected]>
1 parent 725e508 commit e50c2d7

File tree

2 files changed

+162
-6
lines changed

2 files changed

+162
-6
lines changed

Diff for: ‎pkg/bundle/bundle.go

+26-4
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ var ErrUnsupportedMediaType = fmt.Errorf("%w: unsupported media type", ErrValida
3838
var ErrMissingVerificationMaterial = fmt.Errorf("%w: missing verification material", ErrValidation)
3939
var ErrUnimplemented = errors.New("unimplemented")
4040
var ErrInvalidAttestation = fmt.Errorf("%w: invalid attestation", ErrValidation)
41-
var ErrMissingEnvelope = fmt.Errorf("%w: missing envelope", ErrInvalidAttestation)
41+
var ErrMissingEnvelope = fmt.Errorf("%w: missing valid envelope", ErrInvalidAttestation)
4242
var ErrDecodingJSON = fmt.Errorf("%w: decoding json", ErrInvalidAttestation)
4343
var ErrDecodingB64 = fmt.Errorf("%w: decoding base64", ErrInvalidAttestation)
4444

@@ -196,7 +196,7 @@ func validateBundle(b *protobundle.Bundle) error {
196196
switch b.VerificationMaterial.Content.(type) {
197197
case *protobundle.VerificationMaterial_PublicKey, *protobundle.VerificationMaterial_Certificate, *protobundle.VerificationMaterial_X509CertificateChain:
198198
default:
199-
return fmt.Errorf("invalid verififcation material content: verification material must be one of public key, x509 certificate and x509 certificate chain")
199+
return fmt.Errorf("invalid verification material content: verification material must be one of public key, x509 certificate and x509 certificate chain")
200200
}
201201

202202
return nil
@@ -245,8 +245,11 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) {
245245

246246
switch content := b.VerificationMaterial.GetContent().(type) {
247247
case *protobundle.VerificationMaterial_X509CertificateChain:
248+
if content.X509CertificateChain == nil {
249+
return nil, ErrMissingVerificationMaterial
250+
}
248251
certs := content.X509CertificateChain.GetCertificates()
249-
if len(certs) == 0 {
252+
if len(certs) == 0 || certs[0].RawBytes == nil {
250253
return nil, ErrMissingVerificationMaterial
251254
}
252255
parsedCert, err := x509.ParseCertificate(certs[0].RawBytes)
@@ -258,6 +261,9 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) {
258261
}
259262
return cert, nil
260263
case *protobundle.VerificationMaterial_Certificate:
264+
if content.Certificate == nil || content.Certificate.RawBytes == nil {
265+
return nil, ErrMissingVerificationMaterial
266+
}
261267
parsedCert, err := x509.ParseCertificate(content.Certificate.RawBytes)
262268
if err != nil {
263269
return nil, ErrValidationError(err)
@@ -267,6 +273,9 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) {
267273
}
268274
return cert, nil
269275
case *protobundle.VerificationMaterial_PublicKey:
276+
if content.PublicKey == nil {
277+
return nil, ErrMissingVerificationMaterial
278+
}
270279
pk := &PublicKey{
271280
hint: content.PublicKey.Hint,
272281
}
@@ -318,6 +327,9 @@ func (b *Bundle) SignatureContent() (verify.SignatureContent, error) {
318327
}
319328
return envelope, nil
320329
case *protobundle.Bundle_MessageSignature:
330+
if content.MessageSignature == nil || content.MessageSignature.MessageDigest == nil {
331+
return nil, ErrMissingVerificationMaterial
332+
}
321333
return NewMessageSignature(
322334
content.MessageSignature.MessageDigest.Digest,
323335
protocommon.HashAlgorithm_name[int32(content.MessageSignature.MessageDigest.Algorithm)],
@@ -372,11 +384,21 @@ func (b *Bundle) MinVersion(expectVersion string) bool {
372384
}
373385

374386
func parseEnvelope(input *protodsse.Envelope) (*Envelope, error) {
387+
if input == nil {
388+
return nil, ErrMissingEnvelope
389+
}
375390
output := &dsse.Envelope{}
376-
output.Payload = base64.StdEncoding.EncodeToString([]byte(input.GetPayload()))
391+
payload := input.GetPayload()
392+
if payload == nil {
393+
return nil, ErrMissingEnvelope
394+
}
395+
output.Payload = base64.StdEncoding.EncodeToString([]byte(payload))
377396
output.PayloadType = string(input.GetPayloadType())
378397
output.Signatures = make([]dsse.Signature, len(input.GetSignatures()))
379398
for i, sig := range input.GetSignatures() {
399+
if sig == nil {
400+
return nil, ErrMissingEnvelope
401+
}
380402
output.Signatures[i].KeyID = sig.GetKeyid()
381403
output.Signatures[i].Sig = base64.StdEncoding.EncodeToString(sig.GetSig())
382404
}

Diff for: ‎pkg/bundle/bundle_test.go

+136-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1"
2727
protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1"
28+
protodsse "github.com/sigstore/protobuf-specs/gen/pb-go/dsse"
2829
rekorv1 "github.com/sigstore/protobuf-specs/gen/pb-go/rekor/v1"
2930
_ "github.com/sigstore/rekor/pkg/types/hashedrekord"
3031
"github.com/stretchr/testify/require"
@@ -669,6 +670,53 @@ func TestVerificationContent(t *testing.T) {
669670
},
670671
wantErr: true,
671672
},
673+
{
674+
name: "certificate chain with nil bytes",
675+
pb: Bundle{
676+
Bundle: &protobundle.Bundle{
677+
VerificationMaterial: &protobundle.VerificationMaterial{
678+
Content: &protobundle.VerificationMaterial_X509CertificateChain{
679+
X509CertificateChain: &protocommon.X509CertificateChain{
680+
Certificates: []*protocommon.X509Certificate{
681+
{
682+
RawBytes: nil,
683+
},
684+
},
685+
},
686+
},
687+
},
688+
},
689+
},
690+
wantErr: true,
691+
},
692+
{
693+
name: "certificate chain with nil cert",
694+
pb: Bundle{
695+
Bundle: &protobundle.Bundle{
696+
VerificationMaterial: &protobundle.VerificationMaterial{
697+
Content: &protobundle.VerificationMaterial_X509CertificateChain{
698+
X509CertificateChain: &protocommon.X509CertificateChain{
699+
Certificates: nil,
700+
},
701+
},
702+
},
703+
},
704+
},
705+
wantErr: true,
706+
},
707+
{
708+
name: "certificate chain with nil chain",
709+
pb: Bundle{
710+
Bundle: &protobundle.Bundle{
711+
VerificationMaterial: &protobundle.VerificationMaterial{
712+
Content: &protobundle.VerificationMaterial_X509CertificateChain{
713+
X509CertificateChain: nil,
714+
},
715+
},
716+
},
717+
},
718+
wantErr: true,
719+
},
672720
{
673721
name: "certificate",
674722
pb: Bundle{
@@ -699,6 +747,36 @@ func TestVerificationContent(t *testing.T) {
699747
},
700748
wantErr: true,
701749
},
750+
{
751+
name: "certificate with nil bytes",
752+
pb: Bundle{
753+
Bundle: &protobundle.Bundle{
754+
VerificationMaterial: &protobundle.VerificationMaterial{
755+
Content: &protobundle.VerificationMaterial_Certificate{
756+
Certificate: &protocommon.X509Certificate{
757+
RawBytes: nil,
758+
},
759+
},
760+
},
761+
},
762+
},
763+
wantErr: true,
764+
},
765+
{
766+
name: "empty certificate",
767+
pb: Bundle{
768+
Bundle: &protobundle.Bundle{
769+
VerificationMaterial: &protobundle.VerificationMaterial{
770+
Content: &protobundle.VerificationMaterial_Certificate{
771+
Certificate: &protocommon.X509Certificate{
772+
RawBytes: nil,
773+
},
774+
},
775+
},
776+
},
777+
},
778+
wantErr: true,
779+
},
702780
{
703781
name: "public key",
704782
pb: Bundle{
@@ -712,6 +790,19 @@ func TestVerificationContent(t *testing.T) {
712790
},
713791
wantPublicKey: true,
714792
},
793+
{
794+
name: "nil public key",
795+
pb: Bundle{
796+
Bundle: &protobundle.Bundle{
797+
VerificationMaterial: &protobundle.VerificationMaterial{
798+
Content: &protobundle.VerificationMaterial_PublicKey{
799+
PublicKey: nil,
800+
},
801+
},
802+
},
803+
},
804+
wantErr: true,
805+
},
715806
}
716807
for _, tt := range tests {
717808
tt := tt
@@ -742,16 +833,50 @@ func TestSignatureContent(t *testing.T) {
742833
pb Bundle
743834
wantEnvelope bool
744835
wantSignature bool
836+
wantErr bool
745837
}{
746838
{
747839
name: "dsse envelope",
748840
pb: Bundle{
749841
Bundle: &protobundle.Bundle{
750-
Content: &protobundle.Bundle_DsseEnvelope{},
842+
Content: &protobundle.Bundle_DsseEnvelope{
843+
DsseEnvelope: &protodsse.Envelope{
844+
Payload: []byte{},
845+
Signatures: []*protodsse.Signature{{Sig: []byte{}, Keyid: ""}},
846+
},
847+
},
751848
},
752849
},
753850
wantEnvelope: true,
754851
},
852+
{
853+
name: "dsse envelope with nil signature",
854+
pb: Bundle{
855+
Bundle: &protobundle.Bundle{
856+
Content: &protobundle.Bundle_DsseEnvelope{
857+
DsseEnvelope: &protodsse.Envelope{
858+
Payload: []byte{},
859+
Signatures: []*protodsse.Signature{nil},
860+
},
861+
},
862+
},
863+
},
864+
wantErr: true,
865+
},
866+
{
867+
name: "dsse envelope with nil payload",
868+
pb: Bundle{
869+
Bundle: &protobundle.Bundle{
870+
Content: &protobundle.Bundle_DsseEnvelope{
871+
DsseEnvelope: &protodsse.Envelope{
872+
Payload: nil,
873+
Signatures: []*protodsse.Signature{{Sig: []byte{}, Keyid: ""}},
874+
},
875+
},
876+
},
877+
},
878+
wantErr: true,
879+
},
755880
{
756881
name: "message signature",
757882
pb: Bundle{
@@ -770,6 +895,10 @@ func TestSignatureContent(t *testing.T) {
770895
tt := tt
771896
t.Run(tt.name, func(t *testing.T) {
772897
got, gotErr := tt.pb.SignatureContent()
898+
if tt.wantErr {
899+
require.Error(t, gotErr)
900+
return
901+
}
773902
require.NoError(t, gotErr)
774903
if tt.wantEnvelope {
775904
require.NotNil(t, got.EnvelopeContent())
@@ -794,7 +923,12 @@ func TestEnvelope(t *testing.T) {
794923
name: "dsse envelope",
795924
pb: Bundle{
796925
Bundle: &protobundle.Bundle{
797-
Content: &protobundle.Bundle_DsseEnvelope{},
926+
Content: &protobundle.Bundle_DsseEnvelope{
927+
DsseEnvelope: &protodsse.Envelope{
928+
Payload: []byte{},
929+
Signatures: []*protodsse.Signature{{Sig: []byte{}, Keyid: ""}},
930+
},
931+
},
798932
},
799933
},
800934
},

0 commit comments

Comments
 (0)