Merge pull request #186 from skalenetwork/bug/potential-segmentation-…

…fault-when-serializing-to-string#180 remove usage of insecure function, add better validation #180
skalenetwork · May 11, 2022 · 669c52c · 669c52c
2 parents 605ee50 + 34544ca
commit 669c52c
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 19 deletions.
diff --git a/bls/BLSSigShare.cpp b/bls/BLSSigShare.cpp
@@ -37,14 +37,12 @@ size_t BLSSigShare::getSignerIndex() const {
 }
 
 std::shared_ptr< std::string > BLSSigShare::toString() {
-    char str[512];
-
     sigShare->to_affine_coordinates();
+    std::string ret = "";
+    ret += libBLS::ThresholdUtils::fieldElementToString( sigShare->X ) + ':' +
+           libBLS::ThresholdUtils::fieldElementToString( sigShare->Y ) + ':' + hint;
 
-    gmp_sprintf( str, "%Nd:%Nd:%s", sigShare->X.as_bigint().data, libff::alt_bn128_Fq::num_limbs,
-        sigShare->Y.as_bigint().data, libff::alt_bn128_Fq::num_limbs, hint.c_str() );
-
-    return std::make_shared< std::string >( str );
+    return std::make_shared< std::string >( ret );
 }
 
 BLSSigShare::BLSSigShare( std::shared_ptr< std::string > _sigShare, size_t _signerIndex,
@@ -116,9 +114,10 @@ BLSSigShare::BLSSigShare( const std::shared_ptr< libff::alt_bn128_G1 >& _sigShar
         throw libBLS::ThresholdUtils::IncorrectInput( "Zero signer index" );
     }
 
-    if ( _hint.length() == 0 ) {
-        throw libBLS::ThresholdUtils::IncorrectInput( "Empty or misformatted hint" );
+    if ( _hint.length() == 0 || _hint.length() > 2 * BLS_MAX_COMPONENT_LEN ) {
+        throw libBLS::ThresholdUtils::IncorrectInput( "Wrong BLS hint" );
     }
+    libBLS::ThresholdUtils::ParseHint( _hint );
 
     if ( !_sigShare->is_well_formed() ) {
         throw libBLS::ThresholdUtils::IsNotWellFormed( "signature is not from G1" );

diff --git a/bls/BLSSignature.cpp b/bls/BLSSignature.cpp
@@ -43,8 +43,8 @@ BLSSignature::BLSSignature( const std::shared_ptr< libff::alt_bn128_G1 > sig, st
     if ( sig->is_zero() ) {
         throw libBLS::ThresholdUtils::IncorrectInput( "Zero BLS signature" );
     }
-    if ( hint.length() == 0 ) {
-        throw libBLS::ThresholdUtils::IncorrectInput( "Empty BLS hint" );
+    if ( _hint.length() == 0 || _hint.length() > 2 * BLS_MAX_COMPONENT_LEN ) {
+        throw libBLS::ThresholdUtils::IncorrectInput( "Wrong BLS hint" );
     }
 }
 
@@ -92,14 +92,12 @@ BLSSignature::BLSSignature(
 }
 
 std::shared_ptr< std::string > BLSSignature::toString() {
-    char str[512];
-
     sig->to_affine_coordinates();
+    std::string ret = "";
+    ret += libBLS::ThresholdUtils::fieldElementToString( sig->X ) + ':' +
+           libBLS::ThresholdUtils::fieldElementToString( sig->Y ) + ':' + hint;
 
-    gmp_sprintf( str, "%Nd:%Nd:%s", sig->X.as_bigint().data, libff::alt_bn128_Fq::num_limbs,
-        sig->Y.as_bigint().data, libff::alt_bn128_Fq::num_limbs, hint.c_str() );
-
-    return std::make_shared< std::string >( str );
+    return std::make_shared< std::string >( ret );
 }
 
 std::string BLSSignature::getHint() const {

diff --git a/bls/bls.h b/bls/bls.h
@@ -34,8 +34,6 @@
 
 #include <libff/algebra/curves/alt_bn128/alt_bn128_pp.hpp>
 
-static constexpr size_t BLS_MAX_COMPONENT_LEN = 80;
-
 static constexpr size_t BLS_MAX_SIG_LEN = 240;
 
 

diff --git a/tools/utils.cpp b/tools/utils.cpp
@@ -307,7 +307,8 @@ std::pair< libff::alt_bn128_Fq, libff::alt_bn128_Fq > ThresholdUtils::ParseHint(
     const std::string& _hint ) {
     auto position = _hint.find( ":" );
 
-    if ( position == std::string::npos ) {
+    if ( position == std::string::npos || position > BLS_MAX_COMPONENT_LEN ||
+         _hint.length() - position - 1 > BLS_MAX_COMPONENT_LEN ) {
         throw IncorrectInput( "Misformatted hint" );
     }
 

diff --git a/tools/utils.h b/tools/utils.h
@@ -32,6 +32,8 @@
 
 #include <libff/algebra/curves/alt_bn128/alt_bn128_pp.hpp>
 
+static constexpr size_t BLS_MAX_COMPONENT_LEN = 77;
+
 namespace libBLS {
 
 class ThresholdUtils {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -34,8 +34,6 @@

		#include <libff/algebra/curves/alt_bn128/alt_bn128_pp.hpp>

		static constexpr size_t BLS_MAX_COMPONENT_LEN = 80;

		static constexpr size_t BLS_MAX_SIG_LEN = 240;


Expand Down