Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
* Merge pull request from GHSA-95pr-fxf5-86gv

An Image may come from an untrusted source and contain an unknown number
of signatures in the .sig manifest. A common pattern in cosign is to use
the number of signatures as the capacity for a new slice. But this means
the size of the slice is based on an unvalidated external input and
could result in cosign running out of memory.

This change adds validation for certain implementations of the
oci.Signatures Get() method to limit the number of image descriptors
returned. This way, callers can rely on the returned slice of signatures
being a reasonable size to process safely.

The limit is set to 1000, which is a generous size based on the
practical restrictions that container registries set for image manifest
size and approximations of memory allocations for signature layers.

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>

* Merge pull request from GHSA-88jx-383q-w4qc

When downloading an attestation or SBOM from an external source, check
its size before reading it into memory. This protects the host from
potentially reading a maliciously large attachment into memory and
exhausting the system.

SBOMs can vary widely in size, and there could be legitimate SBOMs of up
to 700MB. However, reading a 700MB SBOM into memory would easily bring
down a small cloud VM. Moreover, most SBOMs are not going to be that
large. This change sets a reasonable default of 128MiB, and allows
overriding the default by setting the environment variable
`COSIGN_MAX_ATTACHMENT_SIZE`.

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>

---------

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
  • Loading branch information
haydentherapper authored Apr 10, 2024
1 parent 302aee6 commit 629f5f8
Show file tree
Hide file tree
Showing 22 changed files with 657 additions and 7 deletions.
9 changes: 9 additions & 0 deletions cmd/cosign/cli/verify/verify_blob_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/sigstore/cosign/v2/cmd/cosign/cli/options"
"github.com/sigstore/cosign/v2/cmd/cosign/cli/rekor"
internal "github.com/sigstore/cosign/v2/internal/pkg/cosign"
payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size"
"github.com/sigstore/cosign/v2/internal/pkg/cosign/tsa"
"github.com/sigstore/cosign/v2/pkg/blob"
"github.com/sigstore/cosign/v2/pkg/cosign"
Expand Down Expand Up @@ -117,6 +118,14 @@ func (c *VerifyBlobAttestationCommand) Exec(ctx context.Context, artifactPath st
return err
}
defer f.Close()
fileInfo, err := f.Stat()
if err != nil {
return err
}
err = payloadsize.CheckSize(uint64(fileInfo.Size()))
if err != nil {
return err
}

payload = internal.NewHashReader(f, sha256.New())
if _, err := io.ReadAll(&payload); err != nil {
Expand Down
12 changes: 12 additions & 0 deletions cmd/cosign/cli/verify/verify_blob_attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ gZPFIp557+TOoDxf14FODWc+sIPETk0OgCplAk60doVXbCv33IU4rXZHrg==
const (
blobContents = "some-payload"
anotherBlobContents = "another-blob"
hugeBlobContents = "hugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayloadhugepayload"
blobSLSAProvenanceSignature = "eyJwYXlsb2FkVHlwZSI6ImFwcGxpY2F0aW9uL3ZuZC5pbi10b3RvK2pzb24iLCJwYXlsb2FkIjoiZXlKZmRIbHdaU0k2SW1oMGRIQnpPaTh2YVc0dGRHOTBieTVwYnk5VGRHRjBaVzFsYm5RdmRqQXVNU0lzSW5CeVpXUnBZMkYwWlZSNWNHVWlPaUpvZEhSd2N6b3ZMM05zYzJFdVpHVjJMM0J5YjNabGJtRnVZMlV2ZGpBdU1pSXNJbk4xWW1wbFkzUWlPbHQ3SW01aGJXVWlPaUppYkc5aUlpd2laR2xuWlhOMElqcDdJbk5vWVRJMU5pSTZJalkxT0RjNE1XTmtOR1ZrT1dKallUWXdaR0ZqWkRBNVpqZGlZamt4TkdKaU5URTFNREpsT0dJMVpEWXhPV1kxTjJZek9XRXhaRFkxTWpVNU5tTmpNalFpZlgxZExDSndjbVZrYVdOaGRHVWlPbnNpWW5WcGJHUmxjaUk2ZXlKcFpDSTZJaklpZlN3aVluVnBiR1JVZVhCbElqb2llQ0lzSW1sdWRtOWpZWFJwYjI0aU9uc2lZMjl1Wm1sblUyOTFjbU5sSWpwN2ZYMTlmUT09Iiwic2lnbmF0dXJlcyI6W3sia2V5aWQiOiIiLCJzaWciOiJNRVVDSUE4S2pacWtydDkwZnpCb2pTd3d0ajNCcWI0MUU2cnV4UWs5N1RMbnB6ZFlBaUVBek9Bak9Uenl2VEhxYnBGREFuNnpocmc2RVp2N2t4SzVmYVJvVkdZTWgyYz0ifV19"
dssePredicateEmptySubject = "eyJwYXlsb2FkVHlwZSI6ImFwcGxpY2F0aW9uL3ZuZC5pbi10b3RvK2pzb24iLCJwYXlsb2FkIjoiZXlKZmRIbHdaU0k2SW1oMGRIQnpPaTh2YVc0dGRHOTBieTVwYnk5VGRHRjBaVzFsYm5RdmRqQXVNU0lzSW5CeVpXUnBZMkYwWlZSNWNHVWlPaUpvZEhSd2N6b3ZMM05zYzJFdVpHVjJMM0J5YjNabGJtRnVZMlV2ZGpBdU1pSXNJbk4xWW1wbFkzUWlPbHRkTENKd2NtVmthV05oZEdVaU9uc2lZblZwYkdSbGNpSTZleUpwWkNJNklqSWlmU3dpWW5WcGJHUlVlWEJsSWpvaWVDSXNJbWx1ZG05allYUnBiMjRpT25zaVkyOXVabWxuVTI5MWNtTmxJanA3ZlgxOWZRPT0iLCJzaWduYXR1cmVzIjpbeyJrZXlpZCI6IiIsInNpZyI6Ik1FWUNJUUNrTEV2NkhZZ0svZDdUK0N3NTdXbkZGaHFUTC9WalAyVDA5Q2t1dk1nbDRnSWhBT1hBM0lhWWg1M1FscVk1eVU4cWZxRXJma2tGajlEakZnaWovUTQ2NnJSViJ9XX0="
dssePredicateMissingSha256 = "eyJwYXlsb2FkVHlwZSI6ImFwcGxpY2F0aW9uL3ZuZC5pbi10b3RvK2pzb24iLCJwYXlsb2FkIjoiZXlKZmRIbHdaU0k2SW1oMGRIQnpPaTh2YVc0dGRHOTBieTVwYnk5VGRHRjBaVzFsYm5RdmRqQXVNU0lzSW5CeVpXUnBZMkYwWlZSNWNHVWlPaUpvZEhSd2N6b3ZMM05zYzJFdVpHVjJMM0J5YjNabGJtRnVZMlV2ZGpBdU1pSXNJbk4xWW1wbFkzUWlPbHQ3SW01aGJXVWlPaUppYkc5aUlpd2laR2xuWlhOMElqcDdmWDFkTENKd2NtVmthV05oZEdVaU9uc2lZblZwYkdSbGNpSTZleUpwWkNJNklqSWlmU3dpWW5WcGJHUlVlWEJsSWpvaWVDSXNJbWx1ZG05allYUnBiMjRpT25zaVkyOXVabWxuVTI5MWNtTmxJanA3ZlgxOWZRPT0iLCJzaWduYXR1cmVzIjpbeyJrZXlpZCI6IiIsInNpZyI6Ik1FVUNJQysvM2M4RFo1TGFZTEx6SFZGejE3ZmxHUENlZXVNZ2tIKy8wa2s1cFFLUEFpRUFqTStyYnBBRlJybDdpV0I2Vm9BYVZPZ3U3NjRRM0JKdHI1bHk4VEFHczNrPSJ9XX0="
Expand All @@ -46,13 +47,15 @@ func TestVerifyBlobAttestation(t *testing.T) {

blobPath := writeBlobFile(t, td, blobContents, "blob")
anotherBlobPath := writeBlobFile(t, td, anotherBlobContents, "other-blob")
hugeBlobPath := writeBlobFile(t, td, hugeBlobContents, "huge-blob")
keyRef := writeBlobFile(t, td, pubkey, "cosign.pub")

tests := []struct {
description string
blobPath string
signature string
predicateType string
env map[string]string
shouldErr bool
}{
{
Expand Down Expand Up @@ -98,11 +101,20 @@ func TestVerifyBlobAttestation(t *testing.T) {
signature: dssePredicateMultipleSubjectsInvalid,
blobPath: blobPath,
shouldErr: true,
}, {
description: "override file size limit",
signature: blobSLSAProvenanceSignature,
blobPath: hugeBlobPath,
env: map[string]string{"COSIGN_MAX_ATTACHMENT_SIZE": "128"},
shouldErr: true,
},
}

for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
for k, v := range test.env {
t.Setenv(k, v)
}
decodedSig, err := base64.StdEncoding.DecodeString(test.signature)
if err != nil {
t.Fatal(err)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/cyberphone/json-canonicalization v0.0.0-20231011164504-785e29786b46
github.com/depcheck-test/depcheck-test v0.0.0-20220607135614-199033aaa936
github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7
github.com/dustin/go-humanize v1.0.1
github.com/go-openapi/runtime v0.28.0
github.com/go-openapi/strfmt v0.23.0
github.com/go-openapi/swag v0.23.0
Expand Down
31 changes: 31 additions & 0 deletions internal/pkg/cosign/payload/size/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2024 The Sigstore Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package payload

import "fmt"

// MaxLayerSizeExceeded is an error indicating that the layer is too big to read into memory and cosign should abort processing it.
type MaxLayerSizeExceeded struct {
value uint64
maximum uint64
}

func NewMaxLayerSizeExceeded(value, maximum uint64) *MaxLayerSizeExceeded {
return &MaxLayerSizeExceeded{value, maximum}
}

func (e *MaxLayerSizeExceeded) Error() string {
return fmt.Sprintf("size of layer (%d) exceeded the limit (%d)", e.value, e.maximum)
}
38 changes: 38 additions & 0 deletions internal/pkg/cosign/payload/size/size.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2024 The Sigstore Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package payload

import (
"github.com/dustin/go-humanize"
"github.com/sigstore/cosign/v2/pkg/cosign/env"
)

const defaultMaxSize = uint64(134217728) // 128MiB

func CheckSize(size uint64) error {
maxSize := defaultMaxSize
maxSizeOverride, exists := env.LookupEnv(env.VariableMaxAttachmentSize)
if exists {
var err error
maxSize, err = humanize.ParseBytes(maxSizeOverride)
if err != nil {
maxSize = defaultMaxSize
}
}
if size > maxSize {
return NewMaxLayerSizeExceeded(size, maxSize)
}
return nil
}
110 changes: 110 additions & 0 deletions internal/pkg/cosign/payload/size/size_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright 2024 The Sigstore Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package payload

import (
"testing"
)

func TestCheckSize(t *testing.T) {
tests := []struct {
name string
input uint64
setting string
wantErr bool
}{
{
name: "size is within default limit",
input: 1000,
wantErr: false,
},
{
name: "size exceeds default limit",
input: 200000000,
wantErr: true,
},
{
name: "size is within overridden limit (bytes)",
input: 1000,
setting: "1024",
wantErr: false,
},
{
name: "size is exceeds overridden limit (bytes)",
input: 2000,
setting: "1024",
wantErr: true,
},
{
name: "size is within overridden limit (megabytes, short form)",
input: 1999999,
setting: "2M",
wantErr: false,
},
{
name: "size exceeds overridden limit (megabytes, short form)",
input: 2000001,
setting: "2M",
wantErr: true,
},
{
name: "size is within overridden limit (megabytes, long form)",
input: 1999999,
setting: "2MB",
wantErr: false,
},
{
name: "size exceeds overridden limit (megabytes, long form)",
input: 2000001,
setting: "2MB",
wantErr: true,
},
{
name: "size is within overridden limit (mebibytes)",
input: 2097151,
setting: "2MiB",
wantErr: false,
},
{
name: "size exceeds overridden limit (mebibytes)",
input: 2097153,
setting: "2MiB",
wantErr: true,
},
{
name: "size is negative results in default",
input: 5121,
setting: "-5KiB",
wantErr: false,
},
{
name: "invalid setting results in default",
input: 5121,
setting: "five kilobytes",
wantErr: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if test.setting != "" {
t.Setenv("COSIGN_MAX_ATTACHMENT_SIZE", test.setting)
}
got := CheckSize(test.input)
if (got != nil) != (test.wantErr) {
t.Errorf("CheckSize() = %v, expected %v", got, test.wantErr)
}
})
}
}
6 changes: 6 additions & 0 deletions pkg/cosign/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const (
VariablePKCS11ModulePath Variable = "COSIGN_PKCS11_MODULE_PATH"
VariablePKCS11IgnoreCertificate Variable = "COSIGN_PKCS11_IGNORE_CERTIFICATE"
VariableRepository Variable = "COSIGN_REPOSITORY"
VariableMaxAttachmentSize Variable = "COSIGN_MAX_ATTACHMENT_SIZE"

// Sigstore environment variables
VariableSigstoreCTLogPublicKeyFile Variable = "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE"
Expand Down Expand Up @@ -113,6 +114,11 @@ var (
Expects: "string with a repository",
Sensitive: false,
},
VariableMaxAttachmentSize: {
Description: "maximum attachment size to download (default 128MiB)",
Expects: "human-readable unit of memory, e.g. 5120, 20K, 3M, 45MiB, 1GB",
Sensitive: false,
},

VariableSigstoreCTLogPublicKeyFile: {
Description: "overrides what is used to validate the SCT coming back from Fulcio",
Expand Down
31 changes: 31 additions & 0 deletions pkg/oci/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2024 The Sigstore Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package oci

import "fmt"

// MaxLayersExceeded is an error indicating that the artifact has too many layers and cosign should abort processing it.
type MaxLayersExceeded struct {
value int64
maximum int64
}

func NewMaxLayersExceeded(value, maximum int64) *MaxLayersExceeded {
return &MaxLayersExceeded{value, maximum}
}

func (e *MaxLayersExceeded) Error() string {
return fmt.Sprintf("number of layers (%d) exceeded the limit (%d)", e.value, e.maximum)
}
9 changes: 9 additions & 0 deletions pkg/oci/internal/signature/layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

v1 "github.com/google/go-containerregistry/pkg/v1"
payloadsize "github.com/sigstore/cosign/v2/internal/pkg/cosign/payload/size"
"github.com/sigstore/cosign/v2/pkg/cosign/bundle"
"github.com/sigstore/cosign/v2/pkg/oci"
"github.com/sigstore/sigstore/pkg/cryptoutils"
Expand Down Expand Up @@ -58,6 +59,14 @@ func (s *sigLayer) Annotations() (map[string]string, error) {

// Payload implements oci.Signature
func (s *sigLayer) Payload() ([]byte, error) {
size, err := s.Layer.Size()
if err != nil {
return nil, err
}
err = payloadsize.CheckSize(uint64(size))
if err != nil {
return nil, err
}
// Compressed is a misnomer here, we just want the raw bytes from the registry.
r, err := s.Layer.Compressed()
if err != nil {
Expand Down
Loading

0 comments on commit 629f5f8

Please sign in to comment.