Merge pull request algorand#16 from nullun/chore/deterministic-length-validation

Harden length validation for compressed det1024 signatures

Author: jannotti

deterministic.c

@@ -22,7 +22,7 @@ int falcon_det1024_keygen(shake256_context *rng, void *privkey, void *pubkey) {
 }
 
 // Domain separator used to construct the fixed versioned salt string.
-uint8_t falcon_det1024_salt_rest[38] = {"FALCON_DET"};
+static const uint8_t falcon_det1024_salt_rest[38] = {"FALCON_DET"};
 
 // Construct the fixed salt for a given version.
 void falcon_det1024_write_salt(uint8_t dst[40], uint8_t salt_version) {
@@ -85,6 +85,10 @@ int falcon_det1024_convert_compressed_to_ct(void *sig_ct,
 	int16_t coeffs[1 << FALCON_DET1024_LOGN];
 	size_t v;
 
+	if (sig_compressed_len < 2) {
+		return FALCON_ERR_BADSIG;
+	}
+
 	if (((uint8_t*)sig_compressed)[0] != FALCON_DET1024_SIG_COMPRESSED_HEADER) {
 		return FALCON_ERR_BADSIG;
 	}
@@ -95,6 +99,12 @@ int falcon_det1024_convert_compressed_to_ct(void *sig_ct,
 		return FALCON_ERR_SIZE;
 	}
 
+	// Reject trailing bytes, matching the exact-consumption check
+	// that falcon_verify applies to compressed signatures.
+	if (v != sig_compressed_len-2) {
+		return FALCON_ERR_BADSIG;
+	}
+
 	uint8_t *sig = sig_ct;
 	sig[0] = FALCON_DET1024_SIG_CT_HEADER;
 	sig[1] = ((uint8_t*)sig_compressed)[1]; // Copy the salt_version byte.
@@ -133,12 +143,11 @@ int falcon_det1024_verify_compressed(const void *sig, size_t sig_len,
 	}
 
 	// Add back the salt; drop the version byte.
-	size_t salted_sig_len = sig_len + 40 - 1;
-
-	if (salted_sig_len > FALCON_DET1024_SALTED_SIG_COMPRESSED_MAXSIZE){
+	if (sig_len - 1 > FALCON_DET1024_SALTED_SIG_COMPRESSED_MAXSIZE - 40) {
 		return FALCON_ERR_BADSIG;
 	}
 
+	size_t salted_sig_len = sig_len + 40 - 1;
 
 	falcon_det1024_resalt(salted_sig, sig, sig_len);
 

falcon.go

@@ -119,6 +119,10 @@ func (sk *PrivateKey) SignCompressed(msg []byte) (CompressedSignature, error) {
 func (sig *CompressedSignature) ConvertToCT() (CTSignature, error) {
 	sigCT := CTSignature{}
 
+	if len(*sig) < 2 {
+		return CTSignature{}, fmt.Errorf("signature too short: %w", ErrConvertFail)
+	}
+
 	r := C.falcon_det1024_convert_compressed_to_ct(unsafe.Pointer(&sigCT[0]), unsafe.Pointer(&(*sig)[0]), C.size_t(len(*sig)))
 	if r != 0 {
 		return CTSignature{}, fmt.Errorf("error code %d: %w", int(r), ErrConvertFail)

falcon_test.go

@@ -421,3 +421,47 @@ func BenchmarkFalconVerify(b *testing.B) {
 		pk.Verify(sigs[i], strs[i][:])
 	}
 }
+
+func TestFalconMalformedSignatures(t *testing.T) {
+	seed := make([]byte, 64)
+	rand.Read(seed)
+
+	pub, priv, err := GenerateKey(seed)
+	if err != nil {
+		t.Fatalf("failed to generate keys. err message: %s", err)
+	}
+
+	msg := make([]byte, 64)
+	rand.Read(msg)
+
+	sig, err := priv.SignCompressed(msg)
+	if err != nil {
+		t.Fatalf("failed to sign message. err message: %s", err)
+	}
+
+	// A signature shorter than the 2-byte header and salt-version prefix must be rejected.
+	for _, short := range []CompressedSignature{nil, {}, sig[:0], sig[:1]} {
+		err = pub.Verify(short, msg)
+		if err == nil {
+			t.Fatalf("expected verify to fail on %d-byte signature", len(short))
+		}
+
+		_, err = short.ConvertToCT()
+		if err == nil {
+			t.Fatalf("expected ConvertToCT to fail on %d-byte signature", len(short))
+		}
+	}
+
+	// A valid signature with trailing bytes appended must be rejected.
+	trailing := append(append(CompressedSignature{}, sig...), 0)
+
+	err = pub.Verify(trailing, msg)
+	if err == nil {
+		t.Fatalf("expected verify to fail on signature with trailing bytes")
+	}
+
+	_, err = trailing.ConvertToCT()
+	if err == nil {
+		t.Fatalf("expected ConvertToCT to fail on signature with trailing bytes")
+	}
+}

tests/test_deterministic.c

@@ -62,7 +62,7 @@ hextobin(uint8_t *buf, size_t max_len, const char *src)
 	}
 }
 
-uint8_t sigs_ct[NUM_KATS][FALCON_DET1024_SIG_CT_SIZE];
+static uint8_t sigs_ct[NUM_KATS][FALCON_DET1024_SIG_CT_SIZE];
 
 void test_inner(size_t data_len) {
 	uint8_t pubkey[FALCON_DET1024_PUBKEY_SIZE];