From 623c344412eaa99d009ae7ee9d9749b9d434cffc Mon Sep 17 00:00:00 2001 From: kevin Heifner Date: Mon, 18 May 2026 16:03:37 -0500 Subject: [PATCH 1/2] crypto: add try_recover_key; handle the recover_key -1 sentinel The host recover_key intrinsic returns the required public-key size on success and a negative code on a contract-observable failure (bad/empty/truncated signature, unactivated or unknown variant, recovery-math failure). The CDT wrappers never accounted for the negative code: - recover_key assigned the int result straight into size_t, so a -1 became SIZE_MAX, fell into the large-buffer branch, and ran malloc(SIZE_MAX) + a datastream over an invalid pointer -- a WASM trap on any unrecoverable signature. Add try_recover_key, returning std::optional: std::nullopt when the host signals failure, the key otherwise. Intended for callers that accept untrusted, user-submitted signatures and must not abort the transaction on bad input (e.g. consensus inline-action handlers). Harden the by-value recover_key: a negative host code is now a clean check() abort instead of the SIZE_MAX path. Both functions share one recover_key_impl so the recovery/large-buffer logic has a single implementation. The large-buffer branch now also checks the allocation result and the refill return code instead of trusting them blindly. crypto_tests gains try_recover_key_test, which stubs the host recover_key via set_intrinsic and exercises both wrappers end to end: a non-negative result yields the recovered key; a negative result yields std::nullopt from try_recover_key and a clean abort from recover_key. The recovered-key check is guarded so a regression reports a clean failure rather than dereferencing an empty optional. The host-side non-throwing behavior shipped in wire-sysio (recover_key returns -1 on contract-observable failure); this is the CDT-side half that lets contracts actually consume it without trapping. The host's subjective WebAuthn variable-size guard still rejects the transaction before the contract observes anything and is deliberately not maskable. --- libraries/sysiolib/core/sysio/crypto.hpp | 31 ++++++++ libraries/sysiolib/crypto.cpp | 94 +++++++++++++++++------- tests/unit/crypto_tests.cpp | 52 +++++++++++++ 3 files changed, 150 insertions(+), 27 deletions(-) diff --git a/libraries/sysiolib/core/sysio/crypto.hpp b/libraries/sysiolib/core/sysio/crypto.hpp index 42e73bb4d..c331e6aa2 100644 --- a/libraries/sysiolib/core/sysio/crypto.hpp +++ b/libraries/sysiolib/core/sysio/crypto.hpp @@ -9,6 +9,7 @@ #include "serialize.hpp" #include +#include namespace sysio { @@ -446,6 +447,10 @@ namespace sysio { /** * Calculates the public key used for a given signature on a given digest. * + * Aborts the transaction if the signature is malformed or cannot be + * recovered. Use @ref try_recover_key instead when the signature bytes + * are untrusted and the caller must not abort on bad input. + * * @ingroup crypto * @param digest - Digest of the message that was signed * @param sig - Signature @@ -453,6 +458,32 @@ namespace sysio { */ sysio::public_key recover_key( const sysio::checksum256& digest, const sysio::signature& sig ); + /** + * Recovers the public key for a signature over a digest without aborting + * the transaction on a malformed or unrecoverable signature. + * + * Wraps the same host `recover_key` intrinsic as @ref recover_key, but + * treats the host's contract-observable failure code (bad / empty / + * truncated signature, unactivated or unknown signature variant, or + * recovery-math failure) as a recoverable result. Intended for callers + * that accept untrusted, user-submitted signatures and must not halt the + * transaction on bad input (for example consensus inline-action + * handlers). + * + * @ingroup crypto + * @param digest - Digest of the message that was signed + * @param sig - Signature + * @return the recovered public key, or `std::nullopt` if the signature + * could not be recovered + * + * @note The non-throwing guarantee requires a host whose `recover_key` + * intrinsic returns a negative code on failure rather than + * aborting. The host's subjective WebAuthn variable-size guard is + * deliberately not maskable: it rejects the transaction before the + * contract observes anything. + */ + std::optional try_recover_key( const sysio::checksum256& digest, const sysio::signature& sig ); + /** * Tests a given public key with the recovered public key from digest and signature. * diff --git a/libraries/sysiolib/crypto.cpp b/libraries/sysiolib/crypto.cpp index fbffd46b4..de12da0b5 100644 --- a/libraries/sysiolib/crypto.cpp +++ b/libraries/sysiolib/crypto.cpp @@ -4,8 +4,10 @@ */ #include "core/sysio/crypto.hpp" #include "core/sysio/datastream.hpp" +#include "core/sysio/check.hpp" #include +#include extern "C" { struct __attribute__((aligned (16))) capi_checksum160 { uint8_t hash[20]; }; @@ -90,35 +92,73 @@ namespace sysio { return {hash.hash}; } - sysio::public_key recover_key( const sysio::checksum256& digest, const sysio::signature& sig ) { - auto digest_data = digest.extract_as_byte_array(); - - auto sig_data = sysio::pack(sig); - - char optimistic_pubkey_data[256]; - size_t pubkey_size = ::recover_key( reinterpret_cast(digest_data.data()), - sig_data.data(), sig_data.size(), - optimistic_pubkey_data, sizeof(optimistic_pubkey_data) ); - - sysio::public_key pubkey; - if ( pubkey_size <= sizeof(optimistic_pubkey_data) ) { - sysio::datastream pubkey_ds( optimistic_pubkey_data, pubkey_size ); - pubkey_ds >> pubkey; - } else { - constexpr static size_t max_stack_buffer_size = 512; - void* pubkey_data = (max_stack_buffer_size < pubkey_size) ? malloc(pubkey_size) : alloca(pubkey_size); - - ::recover_key( reinterpret_cast(digest_data.data()), - sig_data.data(), sig_data.size(), - reinterpret_cast(pubkey_data), pubkey_size ); - sysio::datastream pubkey_ds( reinterpret_cast(pubkey_data), pubkey_size ); - pubkey_ds >> pubkey; - - if( max_stack_buffer_size < pubkey_size ) { - free(pubkey_data); + namespace { + /// Shared raw signature-recovery path. Returns the recovered public key, + /// or std::nullopt when the host signals a contract-observable failure + /// via a negative return code (bad / empty / truncated signature, + /// unactivated or unknown signature variant, or recovery-math failure). + /// Handles the host's query-then-fill contract for public keys larger + /// than the optimistic stack buffer. One implementation shared by + /// recover_key (which aborts on failure) and try_recover_key (which + /// surfaces it to the caller). + std::optional + recover_key_impl( const sysio::checksum256& digest, const sysio::signature& sig ) { + auto digest_data = digest.extract_as_byte_array(); + + auto sig_data = sysio::pack(sig); + + char optimistic_pubkey_data[256]; + // `::recover_key` returns int: the required public-key size on + // success, or a negative code on a contract-observable failure. + // Capturing it as signed first is essential -- assigning the -1 + // sentinel straight into a size_t yields SIZE_MAX and walks the + // large-buffer path with a bogus length. + int rc = ::recover_key( reinterpret_cast(digest_data.data()), + sig_data.data(), sig_data.size(), + optimistic_pubkey_data, sizeof(optimistic_pubkey_data) ); + if ( rc < 0 ) + return std::nullopt; + + size_t pubkey_size = static_cast(rc); + sysio::public_key pubkey; + if ( pubkey_size <= sizeof(optimistic_pubkey_data) ) { + sysio::datastream pubkey_ds( optimistic_pubkey_data, pubkey_size ); + pubkey_ds >> pubkey; + } else { + constexpr static size_t max_stack_buffer_size = 512; + void* pubkey_data = (max_stack_buffer_size < pubkey_size) ? malloc(pubkey_size) : alloca(pubkey_size); + // malloc can fail; alloca cannot return null. A failed allocation + // is an environment failure (not a bad-signature condition), so it + // aborts rather than masquerading as an unrecoverable signature. + sysio::check( pubkey_data != nullptr, "recover_key: public-key buffer allocation failed" ); + + int refill_rc = ::recover_key( reinterpret_cast(digest_data.data()), + sig_data.data(), sig_data.size(), + reinterpret_cast(pubkey_data), pubkey_size ); + // The first call already succeeded, so the refill must report the + // same size; anything else is an inconsistent host rather than a + // contract-observable failure. + sysio::check( refill_rc >= 0 && static_cast(refill_rc) == pubkey_size, + "recover_key: inconsistent public-key size on refill" ); + sysio::datastream pubkey_ds( reinterpret_cast(pubkey_data), pubkey_size ); + pubkey_ds >> pubkey; + + if( max_stack_buffer_size < pubkey_size ) { + free(pubkey_data); + } } + return pubkey; } - return pubkey; + } // namespace + + sysio::public_key recover_key( const sysio::checksum256& digest, const sysio::signature& sig ) { + auto recovered = recover_key_impl( digest, sig ); + sysio::check( recovered.has_value(), "unable to recover public key from signature" ); + return *recovered; + } + + std::optional try_recover_key( const sysio::checksum256& digest, const sysio::signature& sig ) { + return recover_key_impl( digest, sig ); } void assert_recover_key( const sysio::checksum256& digest, const sysio::signature& sig, const sysio::public_key& pubkey ) { diff --git a/tests/unit/crypto_tests.cpp b/tests/unit/crypto_tests.cpp index 14577a443..95d114faf 100644 --- a/tests/unit/crypto_tests.cpp +++ b/tests/unit/crypto_tests.cpp @@ -6,6 +6,9 @@ #include #include +#include +#include + using sysio::public_key; using sysio::signature; using namespace sysio::native; @@ -36,6 +39,54 @@ SYSIO_TEST_BEGIN(signature_type_test) CHECK_EQUAL( (signature(std::in_place_index<0>, std::array{}) != signature(std::in_place_index<0>, std::array{})), false ) SYSIO_TEST_END +// Exercises the recover_key / try_recover_key wrappers end to end against a +// stubbed host intrinsic (set_intrinsic): wrapper -> ::recover_key -> the +// native intrinsic table -> datastream unpack. Pins the host return-code +// contract the wrappers depend on: a non-negative result is the recovered +// public key's packed size; a negative result is a contract-observable +// failure. recover_key aborts on failure; try_recover_key surfaces it as +// std::nullopt without trapping. +SYSIO_TEST_BEGIN(try_recover_key_test) + // Deterministic K1 public key (variant index 0) round-tripped through the + // stub so the recovered key is checkable without a real secp256k1 vector. + std::array raw{}; + for ( size_t i = 0; i < raw.size(); ++i ) raw[i] = static_cast(i + 1); + const public_key expected( std::in_place_index<0>, raw ); + const std::vector packed = sysio::pack( expected ); + + const sysio::checksum256 digest; // host stubbed; contents irrelevant + const signature sig( std::in_place_index<0>, std::array{} ); + + // --- success: host returns the packed key and its size --- + intrinsics::set_intrinsic( + [&]( const capi_checksum256*, const char*, size_t, char* pub, size_t publen ) -> int { + if ( pub != nullptr && publen >= packed.size() ) + std::memcpy( pub, packed.data(), packed.size() ); + return static_cast( packed.size() ); + } ); + { + auto recovered = sysio::try_recover_key( digest, sig ); + CHECK_EQUAL( recovered.has_value(), true ) + // CHECK_EQUAL is non-fatal; guard the deref so a regression reports a + // clean failure instead of dereferencing an empty optional (UB). + if ( recovered ) + CHECK_EQUAL( (*recovered == expected), true ) + CHECK_EQUAL( (sysio::recover_key( digest, sig ) == expected), true ) + } + + // --- failure: host signals a contract-observable failure (rc < 0) --- + intrinsics::set_intrinsic( + []( const capi_checksum256*, const char*, size_t, char*, size_t ) -> int { + return -1; + } ); + { + auto recovered = sysio::try_recover_key( digest, sig ); + CHECK_EQUAL( recovered.has_value(), false ) // clean nullopt, no trap + CHECK_ASSERT( "unable to recover public key from signature", + ([&](){ sysio::recover_key( digest, sig ); }) ) + } +SYSIO_TEST_END + int main(int argc, char* argv[]) { bool verbose = false; if( argc >= 2 && std::strcmp( argv[1], "-v" ) == 0 ) { @@ -45,5 +96,6 @@ int main(int argc, char* argv[]) { SYSIO_TEST(public_key_type_test) SYSIO_TEST(signature_type_test) + SYSIO_TEST(try_recover_key_test) return has_failed(); } From 667ac479784e7e3288963c09a51def02ea1af5cb Mon Sep 17 00:00:00 2001 From: kevin Heifner Date: Mon, 18 May 2026 17:05:29 -0500 Subject: [PATCH 2/2] crypto: copy digest into an aligned capi_checksum256 capi_checksum256 is declared alignas(16); the byte-array storage from checksum256::extract_as_byte_array() is alignment 1. reinterpret_cast'ing it to capi_checksum256* is a misaligned-pointer access (UB; can trap or miscompile on strict / native targets, and the native unit tests now exercise this path). recover_key_impl and assert_recover_key now memcpy the digest into a properly aligned local and pass that to the host intrinsics. --- libraries/sysiolib/crypto.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/libraries/sysiolib/crypto.cpp b/libraries/sysiolib/crypto.cpp index de12da0b5..27694a150 100644 --- a/libraries/sysiolib/crypto.cpp +++ b/libraries/sysiolib/crypto.cpp @@ -107,13 +107,22 @@ namespace sysio { auto sig_data = sysio::pack(sig); + // capi_checksum256 is declared alignas(16); digest_data is byte-array + // storage (alignment 1). reinterpret_cast'ing its data to + // capi_checksum256* is a misaligned-pointer access -- UB that can + // trap or be miscompiled on strict / native targets (this path is + // also exercised natively by the unit tests). Copy into a properly + // aligned local and pass that instead. + capi_checksum256 capi_digest{}; + std::memcpy( capi_digest.hash, digest_data.data(), sizeof(capi_digest.hash) ); + char optimistic_pubkey_data[256]; // `::recover_key` returns int: the required public-key size on // success, or a negative code on a contract-observable failure. // Capturing it as signed first is essential -- assigning the -1 // sentinel straight into a size_t yields SIZE_MAX and walks the // large-buffer path with a bogus length. - int rc = ::recover_key( reinterpret_cast(digest_data.data()), + int rc = ::recover_key( &capi_digest, sig_data.data(), sig_data.size(), optimistic_pubkey_data, sizeof(optimistic_pubkey_data) ); if ( rc < 0 ) @@ -132,7 +141,7 @@ namespace sysio { // aborts rather than masquerading as an unrecoverable signature. sysio::check( pubkey_data != nullptr, "recover_key: public-key buffer allocation failed" ); - int refill_rc = ::recover_key( reinterpret_cast(digest_data.data()), + int refill_rc = ::recover_key( &capi_digest, sig_data.data(), sig_data.size(), reinterpret_cast(pubkey_data), pubkey_size ); // The first call already succeeded, so the refill must report the @@ -167,7 +176,14 @@ namespace sysio { auto sig_data = sysio::pack(sig); auto pubkey_data = sysio::pack(pubkey); - ::assert_recover_key( reinterpret_cast(digest_data.data()), + // capi_checksum256 is alignas(16); digest_data is byte-array storage + // (alignment 1). Copy into a properly aligned local rather than + // reinterpret_cast'ing through a misaligned pointer (UB; can trap on + // strict / native targets). + capi_checksum256 capi_digest{}; + std::memcpy( capi_digest.hash, digest_data.data(), sizeof(capi_digest.hash) ); + + ::assert_recover_key( &capi_digest, sig_data.data(), sig_data.size(), pubkey_data.data(), pubkey_data.size() ); }