Skip to content

Commit cf42ace

Browse files
author
Ceridwen Coghlan
authored
Merge pull request from GHSA-2h5h-59f5-c5x9
* check size of archive metadata files before read Signed-off-by: Ceridwen Coghlan <[email protected]> * fix your ints Signed-off-by: Ceridwen Coghlan <[email protected]> * unit tests for metadata size errors Signed-off-by: Ceridwen Coghlan <[email protected]> * update for other tests Signed-off-by: Ceridwen Coghlan <[email protected]> * quick checks and comments Signed-off-by: Ceridwen Coghlan <[email protected]> * checks + comments take 2 Signed-off-by: Ceridwen Coghlan <[email protected]> --------- Signed-off-by: Ceridwen Coghlan <[email protected]>
1 parent 46ac0b2 commit cf42ace

File tree

5 files changed

+94
-0
lines changed

5 files changed

+94
-0
lines changed

‎cmd/rekor-server/app/root.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ Memory and file-based signers should only be used for testing.`)
106106
rootCmd.PersistentFlags().StringSlice("enabled_api_endpoints", operationIds, "list of API endpoints to enable using operationId from openapi.yaml")
107107

108108
rootCmd.PersistentFlags().Uint64("max_request_body_size", 0, "maximum size for HTTP request body, in bytes; set to 0 for unlimited")
109+
rootCmd.PersistentFlags().Uint64("max_jar_metadata_size", 1048576, "maximum permitted size for jar META-INF/ files, in bytes; set to 0 for unlimited")
110+
rootCmd.PersistentFlags().Uint64("max_apk_metadata_size", 1048576, "maximum permitted size for apk .SIGN and .PKGINFO files, in bytes; set to 0 for unlimited")
109111

110112
if err := viper.BindPFlags(rootCmd.PersistentFlags()); err != nil {
111113
log.Logger.Fatal(err)

‎pkg/types/alpine/apk.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333

3434
"github.com/sigstore/sigstore/pkg/signature"
3535
"github.com/sigstore/sigstore/pkg/signature/options"
36+
"github.com/spf13/viper"
3637
"gopkg.in/ini.v1"
3738
)
3839

@@ -149,6 +150,12 @@ func (p *Package) Unmarshal(pkgReader io.Reader) error {
149150
}
150151

151152
if strings.HasPrefix(header.Name, ".SIGN") && pkg.Signature == nil {
153+
if header.Size < 0 {
154+
return errors.New("negative header size for .SIGN file")
155+
}
156+
if uint64(header.Size) > viper.GetUint64("max_apk_metadata_size") && viper.GetUint64("max_apk_metadata_size") > 0 {
157+
return fmt.Errorf("uncompressed .SIGN file size %d exceeds max allowed size %d", header.Size, viper.GetUint64("max_apk_metadata_size"))
158+
}
152159
sigBytes := make([]byte, header.Size)
153160
if _, err = sigReader.Read(sigBytes); err != nil && err != io.EOF {
154161
return fmt.Errorf("reading signature: %w", err)
@@ -176,6 +183,12 @@ func (p *Package) Unmarshal(pkgReader io.Reader) error {
176183
}
177184

178185
if header.Name == ".PKGINFO" {
186+
if header.Size < 0 {
187+
return errors.New("negative header size for .PKGINFO file")
188+
}
189+
if uint64(header.Size) > viper.GetUint64("max_apk_metadata_size") && viper.GetUint64("max_apk_metadata_size") > 0 {
190+
return fmt.Errorf("uncompressed .PKGINFO file size %d exceeds max allowed size %d", header.Size, viper.GetUint64("max_apk_metadata_size"))
191+
}
179192
pkginfoContent := make([]byte, header.Size)
180193
if _, err = ctlReader.Read(pkginfoContent); err != nil && err != io.EOF {
181194
return fmt.Errorf("reading .PKGINFO: %w", err)

‎pkg/types/alpine/apk_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@ package alpine
1717

1818
import (
1919
"os"
20+
"strings"
2021
"testing"
2122

2223
"github.com/sigstore/rekor/pkg/pki/x509"
24+
"github.com/spf13/viper"
2325
)
2426

2527
func TestAlpinePackage(t *testing.T) {
28+
2629
inputArchive, err := os.Open("tests/test_alpine.apk")
2730
if err != nil {
2831
t.Fatalf("could not open archive %v", err)
@@ -48,3 +51,22 @@ func TestAlpinePackage(t *testing.T) {
4851
t.Fatalf("signature verification failed: %v", err)
4952
}
5053
}
54+
55+
func TestAlpineMetadataSize(t *testing.T) {
56+
os.Setenv("MAX_APK_METADATA_SIZE", "10")
57+
viper.AutomaticEnv()
58+
59+
inputArchive, err := os.Open("tests/test_alpine.apk")
60+
if err != nil {
61+
t.Fatalf("could not open archive %v", err)
62+
}
63+
64+
p := Package{}
65+
err = p.Unmarshal(inputArchive)
66+
if err == nil {
67+
t.Fatal("expecting metadata too large err")
68+
}
69+
if !strings.Contains(err.Error(), "exceeds max allowed size 10") {
70+
t.Fatalf("unexpected error %v", err)
71+
}
72+
}

‎pkg/types/jar/v0.0.1/entry.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
"github.com/go-openapi/swag"
4646
jarutils "github.com/sassoftware/relic/lib/signjar"
4747
"github.com/sigstore/rekor/pkg/generated/models"
48+
"github.com/spf13/viper"
4849
)
4950

5051
const (
@@ -139,6 +140,20 @@ func (v *V001Entry) fetchExternalEntities(ctx context.Context) (*pkcs7.PublicKey
139140
return nil, nil, types.ValidationError(err)
140141
}
141142

143+
// Checking that uncompressed metadata files are within acceptable bounds before reading into memory.
144+
// Checks match those performed by the relic library in the jarutils.Verify method below. For example,
145+
// the META-INF/MANIFEST.MF is read into memory by the relic lib, but a META-INF/LICENSE file is not.
146+
for _, f := range zipReader.File {
147+
dir, name := path.Split(strings.ToUpper(f.Name))
148+
if dir != "META-INF/" || name == "" || strings.LastIndex(name, ".") < 0 {
149+
continue
150+
}
151+
if f.UncompressedSize64 > viper.GetUint64("max_jar_metadata_size") && viper.GetUint64("max_jar_metadata_size") > 0 {
152+
return nil, nil, types.ValidationError(
153+
fmt.Errorf("uncompressed jar metadata of size %d exceeds max allowed size %d", f.UncompressedSize64, viper.GetUint64("max_jar_metadata_size")))
154+
}
155+
}
156+
142157
// this ensures that the JAR is signed and the signature verifies, as
143158
// well as checks that the hashes in the signed manifest are all valid
144159
jarObjs, err := jarutils.Verify(zipReader, false)

‎pkg/types/jar/v0.0.1/entry_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ import (
2020
"context"
2121
"os"
2222
"reflect"
23+
"strings"
2324
"testing"
2425

2526
"github.com/go-openapi/runtime"
2627
"github.com/go-openapi/strfmt"
2728
"github.com/go-openapi/swag"
2829
"github.com/sigstore/rekor/pkg/generated/models"
2930
"github.com/sigstore/rekor/pkg/types"
31+
"github.com/spf13/viper"
3032
"go.uber.org/goleak"
3133
)
3234

@@ -150,3 +152,43 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ==
150152
}
151153
}
152154
}
155+
156+
func TestJarMetadataSize(t *testing.T) {
157+
type TestCase struct {
158+
caseDesc string
159+
entry V001Entry
160+
expectUnmarshalSuccess bool
161+
expectCanonicalizeSuccess bool
162+
expectedVerifierSuccess bool
163+
}
164+
165+
jarBytes, _ := os.ReadFile("tests/test.jar")
166+
167+
os.Setenv("MAX_JAR_METADATA_SIZE", "10")
168+
viper.AutomaticEnv()
169+
170+
v := V001Entry{
171+
JARModel: models.JarV001Schema{
172+
Archive: &models.JarV001SchemaArchive{
173+
Content: strfmt.Base64(jarBytes),
174+
},
175+
},
176+
}
177+
178+
r := models.Jar{
179+
APIVersion: swag.String(v.APIVersion()),
180+
Spec: v.JARModel,
181+
}
182+
183+
if err := v.Unmarshal(&r); err != nil {
184+
t.Errorf("unexpected unmarshal failure: %v", err)
185+
}
186+
187+
_, err := v.Canonicalize(context.TODO())
188+
if err == nil {
189+
t.Fatal("expecting metadata too large err")
190+
}
191+
if !strings.Contains(err.Error(), "exceeds max allowed size 10") {
192+
t.Fatalf("unexpected error %v", err)
193+
}
194+
}

0 commit comments

Comments
 (0)