bug fixes

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
 		}
 

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
 }
 

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)
+			})
+		})
+	}
+}

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]) {

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 {