From 46077e654a5ec19321c5856ad97410bd8ae81d9a Mon Sep 17 00:00:00 2001 From: kevin Heifner Date: Fri, 18 Sep 2026 11:01:26 -0500 Subject: [PATCH 1/3] bls: check the pairing result before reading it bls_pop_verify and bls_signature_verify call bls_pairing and then compare its output buffer against GT_ONE without checking the return code. The host rejects a point that is not on the curve and returns failure without writing that buffer, so both functions compared whatever the stack slot already held -- a key the host refused to deserialize was accepted or rejected by accident rather than by decision. Check the return code and fail closed, and value-initialize the buffers both functions pass in. --- .../sysiolib/core/sysio/crypto_bls_ext.hpp | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/libraries/sysiolib/core/sysio/crypto_bls_ext.hpp b/libraries/sysiolib/core/sysio/crypto_bls_ext.hpp index 2c2db0eaa..dd1e1e12a 100644 --- a/libraries/sysiolib/core/sysio/crypto_bls_ext.hpp +++ b/libraries/sysiolib/core/sysio/crypto_bls_ext.hpp @@ -476,16 +476,21 @@ namespace detail { std::memcpy(g1_points[1].data(), pubkey.data(), pubkey.size()); g2_fromMessage(pubkey, POP_CIPHERSUITE_ID, g2_points[1]); - bls_gt r; - bls_pairing(g1_points, g2_points, 2, r); + // bls_pairing rejects a point that is not on the curve, and returns without writing res. + // Reading res without checking that compares whatever the buffer already held, so a key + // the host refused to deserialize is accepted or rejected by accident. + bls_gt r{}; + if (bls_pairing(g1_points, g2_points, 2, r) != 0) { + return false; + } return 0 == std::memcmp(r.data(), GT_ONE.data(), GT_ONE.size()); } // pubkey and signature are assumed to be in RAW affine little-endian bytes inline bool bls_signature_verify(const bls_g1& pubkey, const bls_g2& signature_proof, const std::string& msg) { - bls_g1 g1_points[2]; - bls_g2 g2_points[2]; + bls_g1 g1_points[2] = {{0}, {0}}; + bls_g2 g2_points[2] = {{0}, {0}}; std::memcpy(g1_points[0].data(), detail::G1_ONE_NEG.data(), detail::G1_ONE_NEG.size()); std::memcpy(g2_points[0].data(), signature_proof.data(), signature_proof.size()); @@ -493,8 +498,11 @@ namespace detail { std::memcpy(g1_points[1].data(), pubkey.data(), pubkey.size()); detail::g2_fromMessage(msg, detail::CIPHERSUITE_ID, g2_points[1]); - bls_gt r; - bls_pairing(g1_points, g2_points, 2, r); + // See bls_pop_verify: the pairing result must not be read unless the call succeeded. + bls_gt r{}; + if (bls_pairing(g1_points, g2_points, 2, r) != 0) { + return false; + } return 0 == std::memcmp(r.data(), detail::GT_ONE.data(), detail::GT_ONE.size()); } From 102f2089772fa03ca38b9b09dfc03fc7f82a20e1 Mon Sep 17 00:00:00 2001 From: kevin Heifner Date: Fri, 18 Sep 2026 11:52:43 -0500 Subject: [PATCH 2/3] bls: fix the native pairing trampoline and cover both contracts The native bls_pairing trampoline forwarded g1_points for both operands, discarding g2_points and g2_points_len. Every caller that pairs two points -- bls_pop_verify and bls_signature_verify -- handed the host 192 bytes of G2 where 384 are required, so the host rejected the call and native verification of a valid key failed. Checking the pairing return code is what made that visible: before, the rejection was hidden behind the uninitialized comparison. The new unit test registers a stub pairing intrinsic and pins both contracts. It asserts the operand spans the trampoline forwards, and its failure stub deliberately writes a result that would compare equal, so it passes only if the return code is actually read. --- libraries/native/intrinsics.cpp | 2 +- tests/unit/crypto_ext_tests.cpp | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/libraries/native/intrinsics.cpp b/libraries/native/intrinsics.cpp index 5c9c7c307..23ab280af 100644 --- a/libraries/native/intrinsics.cpp +++ b/libraries/native/intrinsics.cpp @@ -761,7 +761,7 @@ int32_t bls_g2_weighted_sum(const char* points, uint32_t points_len, const char* int32_t bls_pairing(const char* g1_points, uint32_t g1_points_len, const char* g2_points, uint32_t g2_points_len, uint32_t n, char* res, uint32_t res_len) { - return intrinsics::get().call(g1_points, g1_points_len, g1_points, g1_points_len, n, res, res_len); + return intrinsics::get().call(g1_points, g1_points_len, g2_points, g2_points_len, n, res, res_len); } int32_t bls_g1_map(const char* e, uint32_t e_len, char* res, uint32_t res_len) diff --git a/tests/unit/crypto_ext_tests.cpp b/tests/unit/crypto_ext_tests.cpp index e61517550..e12537b1b 100644 --- a/tests/unit/crypto_ext_tests.cpp +++ b/tests/unit/crypto_ext_tests.cpp @@ -5,6 +5,9 @@ #include #include +#include + +#include using namespace sysio::native; @@ -82,6 +85,72 @@ SYSIO_TEST_BEGIN(bigint_test) CHECK_EQUAL( (sysio::bigint{chars_256}.size()), 256 ); SYSIO_TEST_END +// Exercises bls_pop_verify / bls_signature_verify end to end against stubbed host +// intrinsics (set_intrinsic): helper -> ::bls_pairing -> the native intrinsic +// table. Two contracts are pinned. +// +// The operand spans the native trampoline forwards. Both helpers pair two points, +// so the host must see 2*96 bytes of G1 against 2*192 bytes of G2. The trampoline +// used to pass the G1 span for both operands, which any real host rejects on size. +// +// The return-code contract. bls_pairing leaves res untouched when it rejects an +// operand, so a non-zero result must short-circuit rather than fall through to +// comparing the buffer -- which is what made the trampoline bug invisible. +SYSIO_TEST_BEGIN(bls_verify_pairing_contract_test) + // g2_fromMessage hashes the input into a G2 point through these three before the + // pairing. Their outputs are irrelevant here; they only have to be reachable. + intrinsics::set_intrinsic( + []( const char*, uint32_t, char*, uint32_t ) -> int32_t { return 0; } ); + intrinsics::set_intrinsic( + []( const char*, uint32_t, char*, uint32_t ) -> int32_t { return 0; } ); + intrinsics::set_intrinsic( + []( const char*, uint32_t, const char*, uint32_t, char*, uint32_t ) -> int32_t { return 0; } ); + + const sysio::bls_g1 pubkey{}; + const sysio::bls_g2 proof{}; + + uint32_t g1_len = 0; + uint32_t g2_len = 0; + uint32_t pairs = 0; + + // --- success: the host reports GT_ONE, so both helpers must verify --- + intrinsics::set_intrinsic( + [&]( const char*, uint32_t g1l, const char*, uint32_t g2l, uint32_t n, char* res, uint32_t res_len ) -> int32_t { + g1_len = g1l; + g2_len = g2l; + pairs = n; + if ( res != nullptr && res_len >= sysio::detail::GT_ONE.size() ) + std::memcpy( res, sysio::detail::GT_ONE.data(), sysio::detail::GT_ONE.size() ); + return 0; + } ); + + const uint32_t expected_g1_len = static_cast( 2 * std::tuple_size::value ); + const uint32_t expected_g2_len = static_cast( 2 * std::tuple_size::value ); + + CHECK_EQUAL( sysio::bls_pop_verify( pubkey, proof ), true ) + CHECK_EQUAL( pairs, 2u ) + CHECK_EQUAL( g1_len, expected_g1_len ) + CHECK_EQUAL( g2_len, expected_g2_len ) + + g1_len = g2_len = pairs = 0; + CHECK_EQUAL( sysio::bls_signature_verify( pubkey, proof, "message" ), true ) + CHECK_EQUAL( pairs, 2u ) + CHECK_EQUAL( g1_len, expected_g1_len ) + CHECK_EQUAL( g2_len, expected_g2_len ) + + // --- failure: the host rejects an operand. It deliberately writes a result that + // WOULD compare equal, so these only pass if the return code is actually read. + intrinsics::set_intrinsic( + []( const char*, uint32_t, const char*, uint32_t, uint32_t, char* res, uint32_t res_len ) -> int32_t { + if ( res != nullptr && res_len >= sysio::detail::GT_ONE.size() ) + std::memcpy( res, sysio::detail::GT_ONE.data(), sysio::detail::GT_ONE.size() ); + return -1; + } ); + + CHECK_EQUAL( sysio::bls_pop_verify( pubkey, proof ), false ) + CHECK_EQUAL( sysio::bls_signature_verify( pubkey, proof, "message" ), false ) +SYSIO_TEST_END + int main(int argc, char* argv[]) { bool verbose = false; if( argc >= 2 && std::strcmp( argv[1], "-v" ) == 0 ) { @@ -93,6 +162,7 @@ int main(int argc, char* argv[]) { SYSIO_TEST(g1_point_test) SYSIO_TEST(g2_point_test) SYSIO_TEST(bigint_test) + SYSIO_TEST(bls_verify_pairing_contract_test) return has_failed(); } From 8e22f1c537761515e1cef4ba0ecf1fe7c7566076 Mon Sep 17 00:00:00 2001 From: kevin Heifner Date: Fri, 18 Sep 2026 12:18:37 -0500 Subject: [PATCH 3/3] bls: assert the pairing operands by content, not only by length The stub recorded operand lengths only, so a trampoline forwarding the G1 pointer with the G2 length satisfied every assertion while handing the host a 384-byte span over a 192-byte object. Fill the key and the proof with distinct bytes and assert that the second G1 point carries the key and the first G2 point carries the proof. --- tests/unit/crypto_ext_tests.cpp | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/unit/crypto_ext_tests.cpp b/tests/unit/crypto_ext_tests.cpp index e12537b1b..d97f389d9 100644 --- a/tests/unit/crypto_ext_tests.cpp +++ b/tests/unit/crypto_ext_tests.cpp @@ -106,37 +106,56 @@ SYSIO_TEST_BEGIN(bls_verify_pairing_contract_test) intrinsics::set_intrinsic( []( const char*, uint32_t, const char*, uint32_t, char*, uint32_t ) -> int32_t { return 0; } ); - const sysio::bls_g1 pubkey{}; - const sysio::bls_g2 proof{}; + // Distinctive, distinct fill so the operands can be told apart by content. Lengths + // alone would not pin the defect: forwarding the G1 pointer with the G2 length + // satisfies every length assertion while handing the host a 384-byte span over a + // 192-byte object. + sysio::bls_g1 pubkey{}; + sysio::bls_g2 proof{}; + pubkey.fill( '\xa5' ); + proof.fill( '\x5c' ); + + constexpr uint32_t g1_size = static_cast( std::tuple_size::value ); + constexpr uint32_t g2_size = static_cast( std::tuple_size::value ); + constexpr uint32_t expected_g1_len = 2 * g1_size; + constexpr uint32_t expected_g2_len = 2 * g2_size; uint32_t g1_len = 0; uint32_t g2_len = 0; uint32_t pairs = 0; + bool g1_carries_pubkey = false; // second G1 point is the public key + bool g2_carries_proof = false; // first G2 point is the signature proof // --- success: the host reports GT_ONE, so both helpers must verify --- intrinsics::set_intrinsic( - [&]( const char*, uint32_t g1l, const char*, uint32_t g2l, uint32_t n, char* res, uint32_t res_len ) -> int32_t { + [&]( const char* g1, uint32_t g1l, const char* g2, uint32_t g2l, uint32_t n, char* res, uint32_t res_len ) -> int32_t { g1_len = g1l; g2_len = g2l; pairs = n; + g1_carries_pubkey = g1 != nullptr && g1l == expected_g1_len + && std::memcmp( g1 + g1_size, pubkey.data(), g1_size ) == 0; + g2_carries_proof = g2 != nullptr && g2l == expected_g2_len + && std::memcmp( g2, proof.data(), g2_size ) == 0; if ( res != nullptr && res_len >= sysio::detail::GT_ONE.size() ) std::memcpy( res, sysio::detail::GT_ONE.data(), sysio::detail::GT_ONE.size() ); return 0; } ); - const uint32_t expected_g1_len = static_cast( 2 * std::tuple_size::value ); - const uint32_t expected_g2_len = static_cast( 2 * std::tuple_size::value ); - CHECK_EQUAL( sysio::bls_pop_verify( pubkey, proof ), true ) CHECK_EQUAL( pairs, 2u ) CHECK_EQUAL( g1_len, expected_g1_len ) CHECK_EQUAL( g2_len, expected_g2_len ) + CHECK_EQUAL( g1_carries_pubkey, true ) + CHECK_EQUAL( g2_carries_proof, true ) g1_len = g2_len = pairs = 0; + g1_carries_pubkey = g2_carries_proof = false; CHECK_EQUAL( sysio::bls_signature_verify( pubkey, proof, "message" ), true ) CHECK_EQUAL( pairs, 2u ) CHECK_EQUAL( g1_len, expected_g1_len ) CHECK_EQUAL( g2_len, expected_g2_len ) + CHECK_EQUAL( g1_carries_pubkey, true ) + CHECK_EQUAL( g2_carries_proof, true ) // --- failure: the host rejects an operand. It deliberately writes a result that // WOULD compare equal, so these only pass if the return code is actually read.