fix: validate BLS threshold identifier lengths

src/dashbls/include/dashbls/threshold.hpp

@@ -11,6 +11,7 @@
 namespace bls {
 
     namespace Threshold {
+        // Identifiers must provide at least 32 bytes; only their first 32 bytes are used.
 
         PrivateKey PrivateKeyShare(const std::vector<PrivateKey>& sks, const Bytes& id);
         PrivateKey PrivateKeyRecover(const std::vector<PrivateKey>& sks, const std::vector<Bytes>& ids);

src/dashbls/rust-bindings/bls-dash-sys/c-bindings/threshold.h

@@ -25,6 +25,8 @@ extern "C" {
 
 const int HashSize = 32;
 
+// Each hash pointer must reference at least HashSize readable bytes.
+
 PrivateKey ThresholdPrivateKeyShare(void** sks, const size_t sksLen, const void* hash, bool* didErr);
 PrivateKey ThresholdPrivateKeyRecover(void** sks,
                                         const size_t sksLen,

src/dashbls/rust-bindings/bls-signatures/src/elements.rs

@@ -6,7 +6,7 @@ use bls_dash_sys::{CoreMPLDeriveChildPkUnhardened, G1ElementFree, G1ElementFromB
 #[cfg(feature = "use_serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
-use crate::{schemes::Scheme, utils::c_err_to_result, BlsError, BasicSchemeMPL};
+use crate::{schemes::Scheme, utils::{c_err_to_result, check_threshold_ids}, BlsError, BasicSchemeMPL};
 
 // TODO Split into modules
 
@@ -115,9 +115,11 @@ impl G1Element {
         self.fingerprint_with_legacy_flag(false)
     }
 
+    /// Identifiers must contain at least 32 bytes; only their first 32 bytes are used.
     pub fn threshold_recover(
         bls_ids_with_elements: &[(Vec<u8>, G1Element)],
     ) -> Result<Self, BlsError> {
+        check_threshold_ids(bls_ids_with_elements)?;
         unsafe {
             let len = bls_ids_with_elements.len();
             let (c_hashes, c_elements): (Vec<_>, Vec<_>) = bls_ids_with_elements
@@ -254,9 +256,11 @@ impl G2Element {
         self.to_bytes_with_legacy_flag(false)
     }
 
+    /// Identifiers must contain at least 32 bytes; only their first 32 bytes are used.
     pub fn threshold_recover(
         bls_ids_with_elements: &[(Vec<u8>, G2Element)],
     ) -> Result<Self, BlsError> {
+        check_threshold_ids(bls_ids_with_elements)?;
         unsafe {
             let len = bls_ids_with_elements.len();
             let (c_hashes, c_elements): (Vec<_>, Vec<_>) = bls_ids_with_elements

src/dashbls/rust-bindings/bls-signatures/src/lib.rs

@@ -37,6 +37,55 @@ mod tests {
     use super::*;
     use crate::schemes::{AugSchemeMPL, Scheme};
 
+    #[test]
+    fn threshold_recovery_identifier_lengths() {
+        let scheme = BasicSchemeMPL::new();
+        let keys = [
+            PrivateKey::key_gen(&scheme, &[1; 32]).unwrap(),
+            PrivateKey::key_gen(&scheme, &[2; 32]).unwrap(),
+        ];
+        let public_keys = [keys[0].g1_element().unwrap(), keys[1].g1_element().unwrap()];
+        let signatures = [scheme.sign(&keys[0], b"message"), scheme.sign(&keys[1], b"message")];
+        let mut ids = [vec![0; 32], vec![0; 32]];
+        ids[0][31] = 1;
+        ids[1][31] = 2;
+        let secret_shares = [(ids[0].clone(), keys[0].clone()), (ids[1].clone(), keys[1].clone())];
+        let public_shares = [(ids[0].clone(), public_keys[0].clone()), (ids[1].clone(), public_keys[1].clone())];
+        let signature_shares = [(ids[0].clone(), signatures[0].clone()), (ids[1].clone(), signatures[1].clone())];
+        let secret = PrivateKey::threshold_recover(&secret_shares).unwrap();
+        let public = G1Element::threshold_recover(&public_shares).unwrap();
+        let signature = G2Element::threshold_recover(&signature_shares).unwrap();
+        assert_eq!(secret.g1_element().unwrap(), public);
+        assert!(scheme.verify(&public, b"message", &signature));
+
+        for length in [0, 1, 31] {
+            for position in 0..2 {
+                let mut short_secret = secret_shares.clone();
+                let mut short_public = public_shares.clone();
+                let mut short_signature = signature_shares.clone();
+                // Truncation keeps initialized 32-byte backing allocations for regression checks.
+                short_secret[position].0.truncate(length);
+                short_public[position].0.truncate(length);
+                short_signature[position].0.truncate(length);
+                assert!(PrivateKey::threshold_recover(&short_secret).is_err());
+                assert!(G1Element::threshold_recover(&short_public).is_err());
+                assert!(G2Element::threshold_recover(&short_signature).is_err());
+            }
+        }
+
+        let mut long_secret = secret_shares;
+        let mut long_public = public_shares;
+        let mut long_signature = signature_shares;
+        for position in 0..2 {
+            long_secret[position].0.push(0xff);
+            long_public[position].0.push(0xff);
+            long_signature[position].0.push(0xff);
+        }
+        assert_eq!(PrivateKey::threshold_recover(&long_secret).unwrap(), secret);
+        assert_eq!(G1Element::threshold_recover(&long_public).unwrap(), public);
+        assert_eq!(G2Element::threshold_recover(&long_signature).unwrap(), signature);
+    }
+
     #[test]
     fn basic_sign() {
         let seed = b"seedweedseedweedseedweedseedweed";

src/dashbls/rust-bindings/bls-signatures/src/private_key.rs

@@ -10,7 +10,7 @@ use rand::{prelude::StdRng, Rng};
 #[cfg(feature = "use_serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
-use crate::{schemes::Scheme, utils::{c_err_to_result, SecureBox}, BasicSchemeMPL, BlsError, G1Element, G2Element};
+use crate::{schemes::Scheme, utils::{c_err_to_result, check_threshold_ids, SecureBox}, BasicSchemeMPL, BlsError, G1Element, G2Element};
 
 pub const PRIVATE_KEY_SIZE: usize = 32; // TODO somehow extract it from bls library
 
@@ -171,9 +171,11 @@ impl PrivateKey {
         }
     }
 
+    /// Identifiers must contain at least 32 bytes; only their first 32 bytes are used.
     pub fn threshold_recover(
         bls_ids_with_private_keys: &[(Vec<u8>, PrivateKey)],
     ) -> Result<Self, BlsError> {
+        check_threshold_ids(bls_ids_with_private_keys)?;
         unsafe {
             let len = bls_ids_with_private_keys.len();
             let (c_hashes, c_elements): (Vec<_>, Vec<_>) = bls_ids_with_private_keys

src/dashbls/rust-bindings/bls-signatures/src/utils.rs

@@ -8,6 +8,15 @@ use bls_dash_sys::{GetLastErrorMsg, SecAllocBytes, SecFree};
 
 use crate::BlsError;
 
+pub(crate) fn check_threshold_ids<T>(shares: &[(Vec<u8>, T)]) -> Result<(), BlsError> {
+    if shares.iter().any(|(id, _)| id.len() < 32) {
+        return Err(BlsError {
+            msg: "Threshold identifier must contain at least 32 bytes".to_owned(),
+        });
+    }
+    Ok(())
+}
+
 pub(crate) fn c_err_to_result<T, F>(f: F) -> Result<T, BlsError>
 where
     F: FnOnce(&mut bool) -> T,

src/dashbls/src/test.cpp

@@ -1437,6 +1437,69 @@ TEST_CASE("Legacy HD keys") {
 }
 
 TEST_CASE("Threshold Signatures") {
+    SECTION("Identifier lengths") {
+        const vector<PrivateKey> coefficients{
+            BasicSchemeMPL().KeyGen(vector<uint8_t>(32, 1)),
+            BasicSchemeMPL().KeyGen(vector<uint8_t>(32, 2)),
+        };
+        const vector<G1Element> publicCoefficients{
+            coefficients[0].GetG1Element(), coefficients[1].GetG1Element(),
+        };
+        const vector<G2Element> signatureCoefficients{
+            coefficients[0].GetG2Element(), coefficients[1].GetG2Element(),
+        };
+        vector<uint8_t> id1(32, 0), id2(32, 0);
+        id1.back() = 1;
+        id2.back() = 2;
+        const vector<Bytes> ids{id1, id2};
+        const vector<PrivateKey> secretShares{
+            Threshold::PrivateKeyShare(coefficients, ids[0]),
+            Threshold::PrivateKeyShare(coefficients, ids[1]),
+        };
+        const vector<G1Element> publicShares{
+            Threshold::PublicKeyShare(publicCoefficients, ids[0]),
+            Threshold::PublicKeyShare(publicCoefficients, ids[1]),
+        };
+        const vector<G2Element> signatureShares{
+            Threshold::SignatureShare(signatureCoefficients, ids[0]),
+            Threshold::SignatureShare(signatureCoefficients, ids[1]),
+        };
+        REQUIRE(secretShares[0].GetG1Element() == publicShares[0]);
+        REQUIRE(secretShares[0].GetG2Element() == signatureShares[0]);
+        REQUIRE(Threshold::PrivateKeyRecover(secretShares, ids) == coefficients[0]);
+        REQUIRE(Threshold::PublicKeyRecover(publicShares, ids) == publicCoefficients[0]);
+        REQUIRE(Threshold::SignatureRecover(signatureShares, ids) == signatureCoefficients[0]);
+
+        for (const size_t size : {0U, 1U, 31U}) {
+            // Keep the backing buffer initialized and full-sized even when the view is short.
+            const Bytes shortId(id1.data(), size);
+            REQUIRE_THROWS_AS(Threshold::PrivateKeyShare(coefficients, shortId), std::invalid_argument);
+            REQUIRE_THROWS_AS(Threshold::PublicKeyShare(publicCoefficients, shortId), std::invalid_argument);
+            REQUIRE_THROWS_AS(Threshold::SignatureShare(signatureCoefficients, shortId), std::invalid_argument);
+            for (size_t position = 0; position < ids.size(); ++position) {
+                vector<Bytes> shortIds;
+                for (size_t i = 0; i < ids.size(); ++i) {
+                    shortIds.emplace_back(ids[i].begin(), i == position ? size : ids[i].size());
+                }
+                REQUIRE_THROWS_AS(Threshold::PrivateKeyRecover(secretShares, shortIds), std::invalid_argument);
+                REQUIRE_THROWS_AS(Threshold::PublicKeyRecover(publicShares, shortIds), std::invalid_argument);
+                REQUIRE_THROWS_AS(Threshold::SignatureRecover(signatureShares, shortIds), std::invalid_argument);
+            }
+        }
+
+        auto longId1 = id1;
+        auto longId2 = id2;
+        longId1.push_back(0xff);
+        longId2.push_back(0xff);
+        const vector<Bytes> longIds{longId1, longId2};
+        REQUIRE(Threshold::PrivateKeyShare(coefficients, longIds[0]) == secretShares[0]);
+        REQUIRE(Threshold::PublicKeyShare(publicCoefficients, longIds[0]) == publicShares[0]);
+        REQUIRE(Threshold::SignatureShare(signatureCoefficients, longIds[0]) == signatureShares[0]);
+        REQUIRE(Threshold::PrivateKeyRecover(secretShares, longIds) == coefficients[0]);
+        REQUIRE(Threshold::PublicKeyRecover(publicShares, longIds) == publicCoefficients[0]);
+        REQUIRE(Threshold::SignatureRecover(signatureShares, longIds) == signatureCoefficients[0]);
+    }
+
     SECTION("Secret Key Shares") {
         size_t m = 3;
         size_t n = 5;

src/dashbls/src/threshold.cpp

@@ -173,6 +173,9 @@ namespace bls {
         if (vecIn.size() < 2) {
             throw std::length_error("At least 2 coefficients required");
         }
+        if (id.size() < Poly::nIdSize) {
+            throw std::invalid_argument("Threshold identifier must contain at least 32 bytes");
+        }
 
         BnGuard x;
         bn_read_bin(x.val, id.begin(), Poly::nIdSize);
@@ -205,6 +208,12 @@ namespace bls {
         */
         const size_t k = vec.size();
 
+        for (const auto& id : ids) {
+            if (id.size() < Poly::nIdSize) {
+                throw std::invalid_argument("Threshold identifier must contain at least 32 bytes");
+            }
+        }
+
         BnArrayGuard delta(k);
         BnArrayGuard ids2(k);