diff --git a/boxer.go b/boxer.go index 50e9867..cb89f02 100644 --- a/boxer.go +++ b/boxer.go @@ -47,8 +47,6 @@ type boxMaker struct { counterparty *rsa.PublicKey } -const maxBoxSize = 4 * 1024 * 1024 - func NewBoxer(key *rsa.PrivateKey, counterparty *rsa.PublicKey) boxMaker { return boxMaker{ key: key, @@ -172,6 +170,10 @@ func (boxer boxMaker) DecodeUnverified(b64 string) (*Box, error) { return nil, fmt.Errorf("decoding base64: %w", err) } + if len(data) > V0MaxSize { + return nil, fmt.Errorf("data too big, is %d, max is %d", len(data), V0MaxSize) + } + return boxer.DecodeRawUnverified(data) } @@ -199,7 +201,7 @@ func (boxer boxMaker) DecodePngUnverified(r io.Reader) (*Box, error) { return nil, fmt.Errorf("decoding png: %w", err) } - if data.Len() > maxBoxSize { + if data.Len() > V0MaxSize { return nil, errors.New("looks to be larger than max box size") } @@ -207,6 +209,10 @@ func (boxer boxMaker) DecodePngUnverified(r io.Reader) (*Box, error) { } func (boxer boxMaker) DecodeRaw(data []byte) (*Box, error) { + if len(data) > V0MaxSize { + return nil, fmt.Errorf("data too big, is %d, max is %d", len(data), V0MaxSize) + } + var outer outerBox if err := msgpack.Unmarshal(data, &outer); err != nil { return nil, fmt.Errorf("unmarshalling outer: %w", err) diff --git a/cross_language_tests/boxer_cross_test.go b/cross_language_tests/boxer_cross_test.go index 1788d68..b08fcf5 100644 --- a/cross_language_tests/boxer_cross_test.go +++ b/cross_language_tests/boxer_cross_test.go @@ -304,3 +304,89 @@ func TestBoxerRuby(t *testing.T) { }) } } + +func TestBoxerMaxSize(t *testing.T) { + t.Parallel() + + // + // Setup keys and similar. + // + aliceKey, err := krypto.RsaRandomKey() + require.NoError(t, err) + var alicePubPem bytes.Buffer + require.NoError(t, krypto.RsaPublicKeyToPem(aliceKey, &alicePubPem)) + + bobKey, err := krypto.RsaRandomKey() + require.NoError(t, err) + var bobPem bytes.Buffer + require.NoError(t, krypto.RsaPrivateKeyToPem(bobKey, &bobPem)) + + malloryKey, err := krypto.RsaRandomKey() + require.NoError(t, err) + var malloryPem bytes.Buffer + require.NoError(t, krypto.RsaPrivateKeyToPem(malloryKey, &malloryPem)) + + aliceBoxer := krypto.NewBoxer(aliceKey, bobKey.Public().(*rsa.PublicKey)) + + tooBigBytes := mkrand(t, krypto.V0MaxSize+1) + tooBigBytesB64 := base64.StdEncoding.EncodeToString(tooBigBytes) + + t.Run("max size enforced in go", func(t *testing.T) { + t.Parallel() + + _, err = aliceBoxer.Decode(tooBigBytesB64) + require.ErrorContains(t, err, "data too big") + + _, err = aliceBoxer.DecodeUnverified(tooBigBytesB64) + require.ErrorContains(t, err, "data too big") + }) + + t.Run("max size enforced in ruby", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + + pngFile := path.Join(dir, ulid.New()+".png") + //#nosec G306 -- Need readable files + require.NoError(t, os.WriteFile(pngFile, []byte(tooBigBytesB64), 0644)) + + tests := []boxerCrossTestCase{ + {Key: bobPem.Bytes(), Counterparty: alicePubPem.Bytes(), Ciphertext: tooBigBytesB64, cmd: "decode"}, + {Key: bobPem.Bytes(), Counterparty: alicePubPem.Bytes(), Ciphertext: tooBigBytesB64, cmd: "decodeunverified"}, + {Key: bobPem.Bytes(), Ciphertext: tooBigBytesB64, cmd: "decodeunverified"}, + {Key: bobPem.Bytes(), Counterparty: alicePubPem.Bytes(), PngFile: pngFile, cmd: "decodepng"}, + } + + for _, tt := range tests { + tt := tt + + t.Run("", func(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" && tt.cmd == "decodepng" { + t.Skip("skip png decode test on windows because ruby library chunky_png is looking for CRLF png signature") + } + + testfile := path.Join(dir, ulid.New()+".msgpack") + rubyout := path.Join(dir, ulid.New()+"ruby-out") + + // + // Setup + // + b, err := msgpack.Marshal(tt) + require.NoError(t, err) + //#nosec G306 -- Need readable files + require.NoError(t, os.WriteFile(testfile, []byte(base64.StdEncoding.EncodeToString(b)), 0644)) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + //#nosec G204 -- No taint on hardcoded input + cmd := exec.CommandContext(ctx, "ruby", boxerRB, tt.cmd, testfile, rubyout) + out, err := cmd.CombinedOutput() + + require.Error(t, err) + require.Contains(t, string(out), "box too large", "actual out: ", string(out)) + }) + } + }) +} diff --git a/cross_language_tests/challenge_cross_test.go b/cross_language_tests/challenge_cross_test.go index 5acc7ef..abe2c7c 100644 --- a/cross_language_tests/challenge_cross_test.go +++ b/cross_language_tests/challenge_cross_test.go @@ -225,6 +225,40 @@ func TestChallenge_GoGenerate_RubyRespond(t *testing.T) { } } +func TestChallenge_MaxSize(t *testing.T) { + t.Parallel() + + tooBigBytes := mkrand(t, krypto.V0MaxSize+1) + + t.Run("max size enforced in go", func(t *testing.T) { + t.Parallel() + _, err := challenge.UnmarshalChallenge(tooBigBytes) + require.ErrorContains(t, err, "exceeds max size", "should get an error due to size") + }) + + t.Run("max size enforced in ruby", func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + rubyPrivateSigningKey := ecdsaKey(t) + + rubyChallengeCmdData := rubyChallengeCmd{ + RubyPrivateSigningKey: privateEcKeyToPem(t, rubyPrivateSigningKey), + } + + out, err := rubyChallengeExec("generate", dir, rubyChallengeCmdData) + require.NoError(t, err, string(out)) + + rubyChallengeCmdData = rubyChallengeCmd{ + ResponsePack: tooBigBytes, + } + + out, err = rubyChallengeExec("open_response_png", dir, rubyChallengeCmdData) + require.Error(t, err, string(out)) + require.Contains(t, string(out), "response too large", "should get an error due to size") + }) +} + func rubyChallengeExec(rubyCmd, dir string, inputData rubyChallengeCmd) ([]byte, error) { testCaseBytes, err := msgpack.Marshal(inputData) if err != nil { diff --git a/lib/krypto/boxer.rb b/lib/krypto/boxer.rb index 9770748..893f537 100644 --- a/lib/krypto/boxer.rb +++ b/lib/krypto/boxer.rb @@ -82,6 +82,11 @@ def decode_unverified(data) end def decode(data, verify: true, raw: false, png: false) + # Limit size to prevent garbage from filling memory + if data.size > MAX_CHALLENGE_SIZE + raise "box too large" + end + data = unpng(data) if png data = Base64.strict_decode64(data) unless raw || png outer = Outer.new(MessagePack.unpack(data).slice(*OUTER_FIELDS.map(&:to_s))) diff --git a/lib/krypto/challenge.rb b/lib/krypto/challenge.rb index 35a0136..e08e34a 100644 --- a/lib/krypto/challenge.rb +++ b/lib/krypto/challenge.rb @@ -5,6 +5,9 @@ require "openssl" module Krypto + # Limit size to prevent garbage from filling memory + MAX_CHALLENGE_SIZE = 4 * 1024 * 1024 + class Challenge def self.generate(signing_key, challenge_id, challenge_data, request_data, timestamp: Time.now) private_encryption_key = RbNaCl::PrivateKey.generate @@ -29,6 +32,11 @@ def self.generate(signing_key, challenge_id, challenge_data, request_data, times end def self.unmarshal(data, png: false, base64: true) + # Limit size to prevent garbage from filling memory + if data.size > MAX_CHALLENGE_SIZE + raise "challenge too large" + end + data = ::Krypto::Png.decode_blob(data) if png data = Base64.strict_decode64(data) if base64 OuterChallenge.new(MessagePack.unpack(data).slice(*OUTER_CHALLENGE_FIELDS.map(&:to_s))) diff --git a/lib/krypto/challenge_response.rb b/lib/krypto/challenge_response.rb index d173eea..0b93100 100644 --- a/lib/krypto/challenge_response.rb +++ b/lib/krypto/challenge_response.rb @@ -7,6 +7,10 @@ module Krypto class ChallengeResponse def self.unmarshal(data, png: false, base64: true) + if data.size > MAX_CHALLENGE_SIZE + raise "response too large" + end + data = ::Krypto::Png.decode_blob(data) if png data = Base64.strict_decode64(data) if base64 diff --git a/pkg/challenge/challenge.go b/pkg/challenge/challenge.go index 8d0e644..0477a1d 100644 --- a/pkg/challenge/challenge.go +++ b/pkg/challenge/challenge.go @@ -144,6 +144,10 @@ func (o *OuterChallenge) RespondPng(signer crypto.Signer, signer2 crypto.Signer, } func UnmarshalChallenge(outerChallengeBytes []byte) (*OuterChallenge, error) { + if len(outerChallengeBytes) > krypto.V0MaxSize { + return nil, fmt.Errorf("challenge exceeds max size: %d, max is %d", len(outerChallengeBytes), krypto.V0MaxSize) + } + var outerChallenge OuterChallenge if err := msgpack.Unmarshal(outerChallengeBytes, &outerChallenge); err != nil { return nil, err diff --git a/pkg/challenge/response.go b/pkg/challenge/response.go index ff6dbba..e8d3b78 100644 --- a/pkg/challenge/response.go +++ b/pkg/challenge/response.go @@ -84,6 +84,10 @@ type InnerResponse struct { } func UnmarshalResponse(outerResponseBytes []byte) (*OuterResponse, error) { + if len(outerResponseBytes) > krypto.V0MaxSize { + return nil, fmt.Errorf("response to large: is %d, max is %d", len(outerResponseBytes), krypto.V0MaxSize) + } + var outerResponse OuterResponse if err := msgpack.Unmarshal(outerResponseBytes, &outerResponse); err != nil { return nil, err diff --git a/pkg/secureenclave/secureenclave.go b/pkg/secureenclave/secureenclave.go index 0991f51..20c9643 100644 --- a/pkg/secureenclave/secureenclave.go +++ b/pkg/secureenclave/secureenclave.go @@ -133,6 +133,9 @@ func findKey(publicKeySha1 []byte) (*ecdsa.PublicKey, error) { func rawToEcdsa(raw []byte) *ecdsa.PublicKey { ecKey := new(ecdsa.PublicKey) ecKey.Curve = elliptic.P256() + // lint here suggestest using ecdh package, but we are using ecdsa key through out the code + // have found a straight forward to go from ecdh.P256().NewPublicKey(raw) -> ecdsa.PublicKey + //nolint:staticcheck ecKey.X, ecKey.Y = elliptic.Unmarshal(ecKey.Curve, raw) return ecKey } @@ -142,6 +145,9 @@ func publicKeyLookUpHash(key *ecdsa.PublicKey) ([]byte, error) { return nil, errors.New("public key has nil XY coordinates") } + // lint here suggestest using ecdh package, but we are using ecdsa key through out the code + // have found a straight forward to go from ecdh.P256().NewPublicKey(raw) -> ecdsa.PublicKey + //nolint:staticcheck keyBytes := elliptic.Marshal(elliptic.P256(), key.X, key.Y) hash := sha1.New() hash.Write(keyBytes) diff --git a/png.go b/png.go index 81920bb..f541c3a 100644 --- a/png.go +++ b/png.go @@ -16,14 +16,14 @@ const ( pixelsInHeader = 2 alphaValue = 0xFF - v0MaxSize = 1 << 24 + // Limit size to prevent garbage from filling memory + V0MaxSize = 4 * 1024 * 1024 ) func ToPng(w io.Writer, data []byte) error { dataSize := len(data) - - if dataSize > v0MaxSize { - return fmt.Errorf("data too big: %d is bigger than %d", dataSize, v0MaxSize) + if dataSize > V0MaxSize { + return fmt.Errorf("data too big: %d is bigger than %d", dataSize, V0MaxSize) } pixelCount := divCeil(len(data), usableBytesPerPixel)