diff --git a/classify_and_decrypt.go b/classify_and_decrypt.go index f05c7e5..e4208e2 100644 --- a/classify_and_decrypt.go +++ b/classify_and_decrypt.go @@ -28,6 +28,13 @@ const ( minLengthToIdentifyBinarySaltpack int = 23 ) +var ( + // Compile armor prefix regexes once for reuse. + armorWhitespaceRegex = regexp.MustCompile("[>\n\r\t ]+") + armorHeaderRegex = regexp.MustCompile("^BEGIN (?:([a-zA-Z0-9]+) )?SALTPACK (" + EncryptionArmorString + "|" + SignedArmorString + "|" + DetachedSignatureArmorString + ") ?\\.([a-zA-Z0-9 ]*)") + armorWordCountRegex = regexp.MustCompile("^([a-zA-Z0-9]+ ?){0,5}$") +) + // IsSaltpackBinary peeks into the provided bufio.Reader to determine whether it encodes a binary saltpack message. // It does not consume any of the reader's bytes (whose buffer length must be at least minLengthToIdentifyBinarySaltpack). It returns a non nil error if the buffer // size of the reader is not large enough (ErrShortSliceOrBuffer), or if the stream does not appear to contain a binary @@ -130,16 +137,12 @@ func IsSaltpackArmored(stream *bufio.Reader) (brand string, msgType MessageType, // rest of the message is well formed. func IsSaltpackArmoredPrefix(pref string) (brand string, messageType MessageType, ver Version, err error) { // replace blocks of characters in the set [>\n\r\t ] with a single space, so that the next regexp is simpler - re := regexp.MustCompile("[>\n\r\t ]+") - s := strings.TrimSpace(re.ReplaceAllString(pref, " ")) - - headerRegExpSt := "^BEGIN (?:([a-zA-Z0-9]+) )?SALTPACK (" + EncryptionArmorString + "|" + SignedArmorString + "|" + DetachedSignatureArmorString + ") ?\\.([a-zA-Z0-9 ]*)" - headerRegExp := regexp.MustCompile(headerRegExpSt) + s := strings.TrimSpace(armorWhitespaceRegex.ReplaceAllString(pref, " ")) - m := headerRegExp.FindStringSubmatch(s) + m := armorHeaderRegex.FindStringSubmatch(s) if len(m) == 0 { // Matches at most five words - if !regexp.MustCompile("^([a-zA-Z0-9]+ ?){0,5}$").MatchString(s) { + if !armorWordCountRegex.MatchString(s) { return "", MessageTypeUnknown, Version{}, ErrNotASaltpackMessage } diff --git a/common.go b/common.go index 979c41e..a13608c 100644 --- a/common.go +++ b/common.go @@ -26,6 +26,10 @@ type encryptionBlockNumber uint64 func codecHandle() *codec.MsgpackHandle { var mh codec.MsgpackHandle mh.WriteExt = true + // Leave MaxInitLen at zero so go-codec uses its element-size-aware default. + // MaxInitLen is a count of elements, not a byte limit; setting it to a large + // byte-looking value can cause multi-gigabyte allocations for slices whose + // elements are larger than one byte. return &mh } diff --git a/decode_validation_test.go b/decode_validation_test.go new file mode 100644 index 0000000..45a94b8 --- /dev/null +++ b/decode_validation_test.go @@ -0,0 +1,63 @@ +// Copyright 2024 Keybase, Inc. All rights reserved. Use of +// this source code is governed by the included BSD license. + +package saltpack + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCodecMaxInitLenUsesAdaptiveDefault(t *testing.T) { + // In go-codec, MaxInitLen is an element count, not a byte limit. Zero uses + // the codec's element-size-aware default, which targets an initial allocation + // of roughly 256 KiB and grows only as input is actually consumed. + require.Zero(t, codecHandle().MaxInitLen) +} + +func TestEncryptionHeadersAllowMoreThanOneThousandReceivers(t *testing.T) { + receivers := make([]receiverKeys, 1001) + + encHeader := EncryptionHeader{ + FormatName: FormatName, + Version: Version2(), + Type: MessageTypeEncryption, + Receivers: receivers, + } + require.NoError(t, encHeader.validate(CheckKnownMajorVersion)) + + signcryptHeader := SigncryptionHeader{ + FormatName: FormatName, + Version: Version2(), + Type: MessageTypeSigncryption, + Receivers: receivers, + } + require.NoError(t, signcryptHeader.validate()) +} + +func TestAuthenticatorCountMismatch(t *testing.T) { + ds := decryptStream{ + version: Version2(), + position: 1, + numReceivers: 2, + payloadKey: new(SymmetricKey), + } + + tests := []struct { + name string + authenticators []payloadAuthenticator + }{ + {name: "missing authenticator", authenticators: make([]payloadAuthenticator, 1)}, + {name: "extra authenticator", authenticators: make([]payloadAuthenticator, 3)}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.NotPanics(t, func() { + _, err := ds.processBlock(nil, test.authenticators, false, 1) + require.Equal(t, ErrBadCiphertext(1), err) + }) + }) + } +} diff --git a/decrypt.go b/decrypt.go index 439f1ff..9ddabb4 100644 --- a/decrypt.go +++ b/decrypt.go @@ -23,6 +23,7 @@ type decryptStream struct { headerHash headerHash macKey macKey position int + numReceivers int mki MessageKeyInfo } @@ -186,6 +187,7 @@ func (ds *decryptStream) processHeader(hdr *EncryptionHeader) error { } ds.version = hdr.Version + ds.numReceivers = len(hdr.Receivers) ephemeralKey := ds.ring.ImportBoxEphemeralKey(hdr.Ephemeral) if ephemeralKey == nil { @@ -270,7 +272,13 @@ func (ds *decryptStream) processBlock(ciphertext []byte, authenticators []payloa nonce := nonceForChunkSecretBox(blockNum) - // Check the authenticator. + // Each receiver has exactly one authenticator in every payload block. + // Validate the complete structure before indexing with our receiver position. + if len(authenticators) != ds.numReceivers { + return nil, ErrBadCiphertext(seqno) + } + + // Check our authenticator. hashToAuthenticate := computePayloadHash(ds.version, ds.headerHash, nonce, ciphertext, isFinal) ourAuthenticator := computePayloadAuthenticator(ds.macKey, hashToAuthenticate) if !ourAuthenticator.Equal(authenticators[ds.position]) { diff --git a/frame.go b/frame.go index 37d55ee..de84b9f 100644 --- a/frame.go +++ b/frame.go @@ -4,7 +4,6 @@ package saltpack import ( - "regexp" "strings" ) @@ -74,8 +73,7 @@ func parseFrame(m string, typ MessageType, hof headerOrFooterMarker) (brand stri // replace blocks of characters in the set [>\n\r\t ] with a single space, so that Go // can easily parse each piece - re := regexp.MustCompile("[>\n\r\t ]+") - s := strings.TrimSpace(re.ReplaceAllString(m, " ")) + s := strings.TrimSpace(armorWhitespaceRegex.ReplaceAllString(m, " ")) sffx := getStringForType(typ) if len(sffx) == 0 {