CVE-2023-30551: Allocation of Resources Without Limits or Throttling (CWE-770)
value: Rekor is an open source software supply chain transparency log. Rekor prior to version 1.1.1 may crash due to out of memory (OOM) conditions caused by reading archive metadata files into memory without checking their sizes first. Verification of a JAR file submitted to Rekor can cause an out of memory crash if files within the META-INF directory of the JAR are sufficiently large. Parsing of an APK file submitted to Rekor can cause an out of memory crash if the .SIGN or .PKGINFO files within the APK are sufficiently large. The OOM crash has been patched in Rekor version 1.1.1. There are no known workarounds.
CWE description: The product allocates a reusable resource or group of resources on behalf of an actor without imposing any restrictions on the size or number of resources that can be allocated, in violation of the intended security policy for that actor.
CVSS v3 base score: 7.5
Published: 2023-05-08T16:15Z
Repository: https://github.com/sigstore/rekor
Language: Go
Files changed: cmd/rekor-server/app/root.go, pkg/types/alpine/apk.go, pkg/types/alpine/apk_test.go, pkg/types/jar/v0.0.1/entry.go, pkg/types/jar/v0.0.1/entry_test.go
Commit date: 2023-05-02 19:44:35 -0400
Commit message: Merge pull request from GHSA-2h5h-59f5-c5x9

* check size of archive metadata files before read

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>

* fix your ints

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>

* unit tests for metadata size errors

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>

* update for other tests

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>

* quick checks and comments

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>

* checks + comments take 2

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>

---------

Signed-off-by: Ceridwen Coghlan <cdriskill@google.com>
Diff: diff --git a/cmd/rekor-server/app/root.go b/cmd/rekor-server/app/root.go
index f1fbdd1..df31336 100644
--- a/cmd/rekor-server/app/root.go
+++ b/cmd/rekor-server/app/root.go
@@ -104,10 +104,12 @@ Memory and file-based signers should only be used for testing.`)
 	rootCmd.PersistentFlags().Int("max_attestation_size", 100*1024, "max size for attestation storage, in bytes")
 
 	rootCmd.PersistentFlags().StringSlice("enabled_api_endpoints", operationIds, "list of API endpoints to enable using operationId from openapi.yaml")
 
 	rootCmd.PersistentFlags().Uint64("max_request_body_size", 0, "maximum size for HTTP request body, in bytes; set to 0 for unlimited")
+	rootCmd.PersistentFlags().Uint64("max_jar_metadata_size", 1048576, "maximum permitted size for jar META-INF/ files, in bytes; set to 0 for unlimited")
+	rootCmd.PersistentFlags().Uint64("max_apk_metadata_size", 1048576, "maximum permitted size for apk .SIGN and .PKGINFO files, in bytes; set to 0 for unlimited")
 
 	if err := viper.BindPFlags(rootCmd.PersistentFlags()); err != nil {
 		log.Logger.Fatal(err)
 	}
 
diff --git a/pkg/types/alpine/apk.go b/pkg/types/alpine/apk.go
index 51e2d61..5007ee4 100644
--- a/pkg/types/alpine/apk.go
+++ b/pkg/types/alpine/apk.go
@@ -31,10 +31,11 @@ import (
 	"io"
 	"strings"
 
 	"github.com/sigstore/sigstore/pkg/signature"
 	"github.com/sigstore/sigstore/pkg/signature/options"
+	"github.com/spf13/viper"
 	"gopkg.in/ini.v1"
 )
 
 type Package struct {
 	Pkginfo           map[string]string // KVP pairs
@@ -147,10 +148,16 @@ func (p *Package) Unmarshal(pkgReader io.Reader) error {
 		} else if err != nil {
 			return fmt.Errorf("getting next entry in tar archive: %w", err)
 		}
 
 		if strings.HasPrefix(header.Name, ".SIGN") && pkg.Signature == nil {
+			if header.Size < 0 {
+				return errors.New("negative header size for .SIGN file")
+			}
+			if uint64(header.Size) > viper.GetUint64("max_apk_metadata_size") && viper.GetUint64("max_apk_metadata_size") > 0 {
+				return fmt.Errorf("uncompressed .SIGN file size %d exceeds max allowed size %d", header.Size, viper.GetUint64("max_apk_metadata_size"))
+			}
 			sigBytes := make([]byte, header.Size)
 			if _, err = sigReader.Read(sigBytes); err != nil && err != io.EOF {
 				return fmt.Errorf("reading signature: %w", err)
 			}
 			// we're not sure whether this is PEM encoded or not, so handle both cases
@@ -174,10 +181,16 @@ func (p *Package) Unmarshal(pkgReader io.Reader) error {
 		} else if err != nil {
 			return fmt.Errorf("getting next entry in tar archive: %w", err)
 		}
 
 		if header.Name == ".PKGINFO" {
+			if header.Size < 0 {
+				return errors.New("negative header size for .PKGINFO file")
+			}
+			if uint64(header.Size) > viper.GetUint64("max_apk_metadata_size") && viper.GetUint64("max_apk_metadata_size") > 0 {
+				return fmt.Errorf("uncompressed .PKGINFO file size %d exceeds max allowed size %d", header.Size, viper.GetUint64("max_apk_metadata_size"))
+			}
 			pkginfoContent := make([]byte, header.Size)
 			if _, err = ctlReader.Read(pkginfoContent); err != nil && err != io.EOF {
 				return fmt.Errorf("reading .PKGINFO: %w", err)
 			}
 
diff --git a/pkg/types/alpine/apk_test.go b/pkg/types/alpine/apk_test.go
index 9225424..4a5983c 100644
--- a/pkg/types/alpine/apk_test.go
+++ b/pkg/types/alpine/apk_test.go
@@ -15,16 +15,19 @@
 
 package alpine
 
 import (
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/sigstore/rekor/pkg/pki/x509"
+	"github.com/spf13/viper"
 )
 
 func TestAlpinePackage(t *testing.T) {
+
 	inputArchive, err := os.Open("tests/test_alpine.apk")
 	if err != nil {
 		t.Fatalf("could not open archive %v", err)
 	}
 
@@ -46,5 +49,24 @@ func TestAlpinePackage(t *testing.T) {
 
 	if err = p.VerifySignature(pub.CryptoPubKey()); err != nil {
 		t.Fatalf("signature verification failed: %v", err)
 	}
 }
+
+func TestAlpineMetadataSize(t *testing.T) {
+	os.Setenv("MAX_APK_METADATA_SIZE", "10")
+	viper.AutomaticEnv()
+
+	inputArchive, err := os.Open("tests/test_alpine.apk")
+	if err != nil {
+		t.Fatalf("could not open archive %v", err)
+	}
+
+	p := Package{}
+	err = p.Unmarshal(inputArchive)
+	if err == nil {
+		t.Fatal("expecting metadata too large err")
+	}
+	if !strings.Contains(err.Error(), "exceeds max allowed size 10") {
+		t.Fatalf("unexpected error %v", err)
+	}
+}
diff --git a/pkg/types/jar/v0.0.1/entry.go b/pkg/types/jar/v0.0.1/entry.go
index 20ae81d..ab85ccd 100644
--- a/pkg/types/jar/v0.0.1/entry.go
+++ b/pkg/types/jar/v0.0.1/entry.go
@@ -43,10 +43,11 @@ import (
 	"github.com/go-openapi/strfmt"
 
 	"github.com/go-openapi/swag"
 	jarutils "github.com/sassoftware/relic/lib/signjar"
 	"github.com/sigstore/rekor/pkg/generated/models"
+	"github.com/spf13/viper"
 )
 
 const (
 	APIVERSION = "0.0.1"
 )
@@ -137,10 +138,24 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*pkcs7.PublicKey
 	zipReader, err := zip.NewReader(bytes.NewReader(b.Bytes()), n)
 	if err != nil {
 		return nil, nil, types.ValidationError(err)
 	}
 
+	// Checking that uncompressed metadata files are within acceptable bounds before reading into memory.
+	// Checks match those performed by the relic library in the jarutils.Verify method below. For example,
+	// the META-INF/MANIFEST.MF is read into memory by the relic lib, but a META-INF/LICENSE file is not.
+	for _, f := range zipReader.File {
+		dir, name := path.Split(strings.ToUpper(f.Name))
+		if dir != "META-INF/" || name == "" || strings.LastIndex(name, ".") < 0 {
+			continue
+		}
+		if f.UncompressedSize64 > viper.GetUint64("max_jar_metadata_size") && viper.GetUint64("max_jar_metadata_size") > 0 {
+			return nil, nil, types.ValidationError(
+				fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size")))
+		}
+	}
+
 	// this ensures that the JAR is signed and the signature verifies, as
 	// well as checks that the hashes in the signed manifest are all valid
 	jarObjs, err := jarutils.Verify(zipReader, false)
 	if err != nil {
 		return nil, nil, types.ValidationError(err)
diff --git a/pkg/types/jar/v0.0.1/entry_test.go b/pkg/types/jar/v0.0.1/entry_test.go
index 8933476..9253a7e 100644
--- a/pkg/types/jar/v0.0.1/entry_test.go
+++ b/pkg/types/jar/v0.0.1/entry_test.go
@@ -18,17 +18,19 @@ package jar
 import (
 	"bytes"
 	"context"
 	"os"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/swag"
 	"github.com/sigstore/rekor/pkg/generated/models"
 	"github.com/sigstore/rekor/pkg/types"
+	"github.com/spf13/viper"
 	"go.uber.org/goleak"
 )
 
 func TestMain(m *testing.M) {
 	goleak.VerifyTestMain(m)
@@ -148,5 +150,45 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ==
 				t.Errorf("%v: expected error for %v, got %v", tc.caseDesc, string(s), err)
 			}
 		}
 	}
 }
+
+func TestJarMetadataSize(t *testing.T) {
+	type TestCase struct {
+		caseDesc                  string
+		entry                     V001Entry
+		expectUnmarshalSuccess    bool
+		expectCanonicalizeSuccess bool
+		expectedVerifierSuccess   bool
+	}
+
+	jarBytes, _ := os.ReadFile("tests/test.jar")
+
+	os.Setenv("MAX_JAR_METADATA_SIZE", "10")
+	viper.AutomaticEnv()
+
+	v := V001Entry{
+		JARModel: models.JarV001Schema{
+			Archive: &models.JarV001SchemaArchive{
+				Content: strfmt.Base64(jarBytes),
+			},
+		},
+	}
+
+	r := models.Jar{
+		APIVersion: swag.String(v.APIVersion()),
+		Spec:       v.JARModel,
+	}
+
+	if err := v.Unmarshal(&r); err != nil {
+		t.Errorf("unexpected unmarshal failure: %v", err)
+	}
+
+	_, err := v.Canonicalize(context.TODO())
+	if err == nil {
+		t.Fatal("expecting metadata too large err")
+	}
+	if !strings.Contains(err.Error(), "exceeds max allowed size 10") {
+		t.Fatalf("unexpected error %v", err)
+	}
+}

Fixed code: 	rootCmd.PersistentFlags().Uint64("max_jar_metadata_size", 1048576, "maximum permitted size for jar META-INF/ files, in bytes; set to 0 for unlimited")
	rootCmd.PersistentFlags().Uint64("max_apk_metadata_size", 1048576, "maximum permitted size for apk .SIGN and .PKGINFO files, in bytes; set to 0 for unlimited")
	"github.com/spf13/viper"
			if header.Size < 0 {
				return errors.New("negative header size for .SIGN file")
			}
			if uint64(header.Size) > viper.GetUint64("max_apk_metadata_size") && viper.GetUint64("max_apk_metadata_size") > 0 {
				return fmt.Errorf("uncompressed .SIGN file size %d exceeds max allowed size %d", header.Size, viper.GetUint64("max_apk_metadata_size"))
			}
			if header.Size < 0 {
				return errors.New("negative header size for .PKGINFO file")
			}
			if uint64(header.Size) > viper.GetUint64("max_apk_metadata_size") && viper.GetUint64("max_apk_metadata_size") > 0 {
				return fmt.Errorf("uncompressed .PKGINFO file size %d exceeds max allowed size %d", header.Size, viper.GetUint64("max_apk_metadata_size"))
			}
	"strings"
	"github.com/spf13/viper"


func TestAlpineMetadataSize(t *testing.T) {
	os.Setenv("MAX_APK_METADATA_SIZE", "10")
	viper.AutomaticEnv()

	inputArchive, err := os.Open("tests/test_alpine.apk")
	if err != nil {
		t.Fatalf("could not open archive %v", err)
	}

	p := Package{}
	err = p.Unmarshal(inputArchive)
	if err == nil {
		t.Fatal("expecting metadata too large err")
	}
	if !strings.Contains(err.Error(), "exceeds max allowed size 10") {
		t.Fatalf("unexpected error %v", err)
	}
}
	"github.com/spf13/viper"
	// Checking that uncompressed metadata files are within acceptable bounds before reading into memory.
	// Checks match those performed by the relic library in the jarutils.Verify method below. For example,
	// the META-INF/MANIFEST.MF is read into memory by the relic lib, but a META-INF/LICENSE file is not.
	for _, f := range zipReader.File {
		dir, name := path.Split(strings.ToUpper(f.Name))
		if dir != "META-INF/" || name == "" || strings.LastIndex(name, ".") < 0 {
			continue
		}
		if f.UncompressedSize64 > viper.GetUint64("max_jar_metadata_size") && viper.GetUint64("max_jar_metadata_size") > 0 {
			return nil, nil, types.ValidationError(
				fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size")))
		}
	}

	"strings"
	"github.com/spf13/viper"

func TestJarMetadataSize(t *testing.T) {
	type TestCase struct {
		caseDesc                  string
		entry                     V001Entry
		expectUnmarshalSuccess    bool
		expectCanonicalizeSuccess bool
		expectedVerifierSuccess   bool
	}

	jarBytes, _ := os.ReadFile("tests/test.jar")

	os.Setenv("MAX_JAR_METADATA_SIZE", "10")
	viper.AutomaticEnv()

	v := V001Entry{
		JARModel: models.JarV001Schema{
			Archive: &models.JarV001SchemaArchive{
				Content: strfmt.Base64(jarBytes),
			},
		},
	}

	r := models.Jar{
		APIVersion: swag.String(v.APIVersion()),
		Spec:       v.JARModel,
	}

	if err := v.Unmarshal(&r); err != nil {
		t.Errorf("unexpected unmarshal failure: %v", err)
	}

	_, err := v.Canonicalize(context.TODO())
	if err == nil {
		t.Fatal("expecting metadata too large err")
	}
	if !strings.Contains(err.Error(), "exceeds max allowed size 10") {
		t.Fatalf("unexpected error %v", err)
	}
}