From 20fc026b0715783ddafc4336c3be389428c3b497 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 1 Sep 2026 00:56:21 +1200 Subject: [PATCH 1/3] script: reject malformed script integers, add tests --- src/Makefile.am | 25 +++++++---- src/ctest/_CMakeLists.txt | 11 +++++ src/ctest/test_script.c | 88 +++++++++++++++++++++++++++++++++++++++ src/script.c | 19 +++++++-- src/script.h | 2 + src/script_int.h | 4 ++ 6 files changed, 138 insertions(+), 11 deletions(-) create mode 100644 src/ctest/test_script.c diff --git a/src/Makefile.am b/src/Makefile.am index 150f7a545..ab7da2699 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -224,12 +224,13 @@ SUBDIRS = secp256k1 endif TESTS = +TESTS_CFLAGS = -I$(top_srcdir) -I$(top_srcdir)/include -I$(srcdir)/ccan -I$(top_srcdir)/src/secp256k1/include $(AM_CFLAGS) noinst_PROGRAMS = if RUN_TESTS TESTS += test_bech32 noinst_PROGRAMS += test_bech32 test_bech32_SOURCES = ctest/test_bech32.c -test_bech32_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) +test_bech32_CFLAGS = $(TESTS_CFLAGS) test_bech32_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_bech32_LDADD += $(PYTHON_LIBS) @@ -237,7 +238,7 @@ endif TESTS += test_psbt noinst_PROGRAMS += test_psbt test_psbt_SOURCES = ctest/test_psbt.c ccan/ccan/str/hex/hex.c -test_psbt_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) -I$(srcdir)/ccan +test_psbt_CFLAGS = $(TESTS_CFLAGS) test_psbt_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_psbt_LDADD += $(PYTHON_LIBS) @@ -245,7 +246,7 @@ endif TESTS += test_psbt_limits noinst_PROGRAMS += test_psbt_limits test_psbt_limits_SOURCES = ctest/test_psbt_limits.c ccan/ccan/str/hex/hex.c -test_psbt_limits_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) -I$(srcdir)/ccan +test_psbt_limits_CFLAGS = $(TESTS_CFLAGS) test_psbt_limits_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_psbt_limits_LDADD += $(PYTHON_LIBS) @@ -254,7 +255,7 @@ if USE_PTHREAD TESTS += test_clear noinst_PROGRAMS += test_clear test_clear_SOURCES = ctest/test_clear.c -test_clear_CFLAGS = -I$(top_srcdir)/include -I$(top_srcdir) -I$(top_srcdir)/src/secp256k1/include $(PTHREAD_CFLAGS) $(AM_CFLAGS) $(NOOPT_CFLAGS) $(NOBUILTIN_CFLAGS) +test_clear_CFLAGS = $(TESTS_CFLAGS) $(PTHREAD_CFLAGS) $(AM_CFLAGS) $(NOOPT_CFLAGS) $(NOBUILTIN_CFLAGS) test_clear_LIBS = $(PTHREAD_LIBS) test_clear_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX @@ -264,15 +265,23 @@ endif TESTS += test_coinselection noinst_PROGRAMS += test_coinselection test_coinselection_SOURCES = ctest/test_coinselection.c -test_coinselection_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) +test_coinselection_CFLAGS = $(TESTS_CFLAGS) test_coinselection_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_coinselection_LDADD += $(PYTHON_LIBS) endif +TESTS += test_script +noinst_PROGRAMS += test_script +test_script_SOURCES = ctest/test_script.c +test_script_CFLAGS = $(TESTS_CFLAGS) +test_script_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ +if PYTHON_MANYLINUX +test_script_LDADD += $(PYTHON_LIBS) +endif TESTS += test_tx noinst_PROGRAMS += test_tx test_tx_SOURCES = ctest/test_tx.c -test_tx_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) +test_tx_CFLAGS = $(TESTS_CFLAGS) test_tx_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_tx_LDADD += $(PYTHON_LIBS) @@ -280,7 +289,7 @@ endif TESTS += test_descriptor noinst_PROGRAMS += test_descriptor test_descriptor_SOURCES = ctest/test_descriptor.c -test_descriptor_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) +test_descriptor_CFLAGS = $(TESTS_CFLAGS) test_descriptor_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_descriptor_LDADD += $(PYTHON_LIBS) @@ -289,7 +298,7 @@ if BUILD_ELEMENTS TESTS += test_elements_tx noinst_PROGRAMS += test_elements_tx test_elements_tx_SOURCES = ctest/test_elements_tx.c -test_elements_tx_CFLAGS = -I$(top_srcdir)/include $(AM_CFLAGS) +test_elements_tx_CFLAGS = $(TESTS_CFLAGS) test_elements_tx_LDADD = $(lib_LTLIBRARIES) @CTEST_EXTRA_STATIC@ if PYTHON_MANYLINUX test_elements_tx_LDADD += $(PYTHON_LIBS) diff --git a/src/ctest/_CMakeLists.txt b/src/ctest/_CMakeLists.txt index 2b0a9e8d6..1b9cc5e71 100644 --- a/src/ctest/_CMakeLists.txt +++ b/src/ctest/_CMakeLists.txt @@ -36,3 +36,14 @@ add_executable(test_psbt_limits test_psbt_limits.c) target_include_directories(test_psbt_limits PRIVATE ${CMAKE_BINARY_DIR}) target_link_libraries(test_psbt_limits PRIVATE wallycore) add_test(test_psbt_limits test_psbt_limits) + +# TODO: Requires static linking or exposing scriptint_from_bytes w/WALLY_INTERNAL_API +#add_executable(test_script test_script.c) +#target_include_directories(test_script PRIVATE ${CMAKE_BINARY_DIR} $) +#target_link_libraries(test_script PRIVATE wallycore) +#add_test(test_script test_script) + +add_executable(test_tx test_tx.c) +target_include_directories(test_tx PRIVATE ${CMAKE_BINARY_DIR}) +target_link_libraries(test_tx PRIVATE wallycore) +add_test(test_tx test_tx) diff --git a/src/ctest/test_script.c b/src/ctest/test_script.c new file mode 100644 index 000000000..82606b173 --- /dev/null +++ b/src/ctest/test_script.c @@ -0,0 +1,88 @@ +#include "config.h" + +#include +#include "script_int.h" +#include +#include +#include +#include + +#define NUM_ELEMS(a) (sizeof(a) / sizeof(a[0])) + +#define B_1(b1) { b1, 0, 0, 0, 0, 0 } +#define B_2(b1, b2) { b1, b2, 0, 0, 0, 0 } +#define B_3(b1, b2, b3) { b1, b2, b3, 0, 0, 0 } +#define B_4(b1, b2, b3, b4) { b1, b2, b3, b4, 0, 0 } +#define B_5(b1, b2, b3, b4, b5) { b1, b2, b3, b4, b5, 0 } +#define B_6(b1, b2, b3, b4, b5, b6) { b1, b2, b3, b4, b5, b6 } + +#ifdef WALLY_EXPORT_ALL +struct scriptint_test { + int64_t expected; + unsigned char bytes[6]; + bool is_valid; + const char* description; +} scriptint_cases[] = { + /* valid encodings, but rejected by scriptint_from_bytes */ + { 0, B_1(0x00), false, "Must use OP_0 instead" }, + { 1, B_2(0x01, 0x01), false, "Must use OP_1 instead" }, + { 16, B_2(0x01, 0x10), false, "Must use OP_16 instead" }, + { -1, B_2(0x01, 0x81), false, "Negative disallowed (-1)"}, + { -127, B_2(0x01, 0xff), false, "Negative disallowed (-127)" }, + { -128, B_3(0x02, 0x80, 0x80), false, "Negative disallowed (-128)" }, + { -255, B_3(0x02, 0xff, 0x80), false, "Negative disallowed (-255)" }, + { -32767, B_3(0x02, 0xff, 0xff), false, "Negative disallowed (-32767)" }, + { -2147483647, B_5(0x04, 0xff, 0xff, 0xff, 0xff), false, "Negative disallowed (-2147483647)" }, + /* valid */ + { 17, B_2(0x01, 0x11), true, "Minimum allowed" }, + { 127, B_2(0x01, 0x7f), true, "Max 1-byte without padding" }, + { 128, B_3(0x02, 0x80, 0x00), true, "Requires trailing 0" }, + { 255, B_3(0x02, 0xff, 0x00), true, "255" }, + { 32767, B_3(0x02, 0xff, 0x7f), true, "Max 2-byte without padding" }, + { 32768, B_4(0x03, 0x00, 0x80, 0x00), true, "Requires trailing 0" }, + { 8388607, B_4(0x03, 0xff, 0xff, 0x7f), true, "Max 3-byte without padding" }, + { 2147483647, B_5(0x04, 0xff, 0xff, 0xff, 0x7f), true, "Max allowed value 0xffffffff" }, + /* invalid encodings */ + { 0, B_2(0x01, 0x00), false, "Non-minimal 0" }, + { 0, B_2(0x01, 0x80), false, "Negative 0" }, + { 1, B_3(0x02, 0x01, 0x00), false, "Zero padded positive int" }, + { -1, B_3(0x02, 0x01, 0x80), false, "Zero padding on negative int" }, + { 127, B_3(0x02, 0x7f, 0x00), false, "Trailing 0" }, + { -127, B_3(0x02, 0x7f, 0x80), false, "Trailing sign byte" }, + { 0, B_4(0x03, 0x00, 0x00, 0x00), false, "Negative Case: Multi-byte redundant zeros" }, + { 0, B_6(0x05, 0x01, 0x02, 0x03, 0x04, + 0x05), false, ">4 bytes" } +}; + +static bool test_scriptint_parse(void) +{ + for (size_t i = 0; i < NUM_ELEMS(scriptint_cases); ++i) { + const struct scriptint_test* test = scriptint_cases + i; + const size_t len = test->bytes[0] + 1; + int64_t v; + int ret = scriptint_from_bytes(test->bytes, len, &v); + if (!test->is_valid) { + if (ret != WALLY_EINVAL) + return false; + } else { + if (ret != WALLY_OK || v != test->expected) + return false; + } + } + + return true; +} +#endif + +int main(void) +{ + bool tests_ok = true; + +#define RUN(t) if (!t()) { printf(#t "() test failed!\n"); tests_ok = false; } + +#ifdef WALLY_EXPORT_ALL + RUN(test_scriptint_parse); +#endif + + return tests_ok ? 0 : 1; +} diff --git a/src/script.c b/src/script.c index c48d5395d..f7c07bf10 100644 --- a/src/script.c +++ b/src/script.c @@ -191,7 +191,7 @@ size_t scriptint_to_bytes(int64_t signed_v, unsigned char *bytes_out) return len; } -int64_t scriptint_from_bytes(const unsigned char *bytes, size_t len, int64_t *value_out) +int scriptint_from_bytes(const unsigned char *bytes, size_t len, int64_t *value_out) { int64_t mask = 0x80; size_t i; @@ -203,10 +203,17 @@ int64_t scriptint_from_bytes(const unsigned char *bytes, size_t len, int64_t *va * This function is intended for parsing scripts, not evaluating them * (which can use intermediate 5 byte script int stack values). */ - if (!bytes || len < 1 || len <= bytes[0] || bytes[0] > 4 || !value_out) + if (!bytes || !len || len <= bytes[0] || bytes[0] > 4 || !value_out) return WALLY_EINVAL; - for (i = 0; i < bytes[0]; ++i) { + len = bytes[0]; /* Set len to the length of the encoded number */ + if ((bytes[len] & 0x7f) == 0) { + /* MSB is non-zero */ + if (len == 1 || (bytes[len - 1] & 0x80) == 0) + return WALLY_EINVAL; /* Non-minimal encoding */ + } + + for (i = 0; i < len; ++i) { *value_out |= (int64_t)(bytes[i + 1]) << (8 * i); mask <<= 8; } @@ -216,6 +223,12 @@ int64_t scriptint_from_bytes(const unsigned char *bytes, size_t len, int64_t *va *value_out ^= (mask >> 8); *value_out = -*value_out; } + /* Reject non-minimal (OP_0->OP_16) and negative numbers. + * There are no meaningful uses for negative numbers in current script + * and they are unsupported in tapscript. + */ + if (*value_out <= 16) + return WALLY_EINVAL; /* reject Non-minimal/negative (should be OP_0->OP_16) */ return WALLY_OK; } diff --git a/src/script.h b/src/script.h index cb39e7dc2..75872c44e 100644 --- a/src/script.h +++ b/src/script.h @@ -2,6 +2,8 @@ #define LIBWALLY_CORE_SCRIPT_INTERNAL_H 1 #include +#include +#include /* Get the size of a push from the script push opcode(s) */ int script_get_push_size_from_bytes( diff --git a/src/script_int.h b/src/script_int.h index d90576c72..badb7296e 100644 --- a/src/script_int.h +++ b/src/script_int.h @@ -2,6 +2,8 @@ #define LIBWALLY_CORE_SCRIPT_INT_H 1 #include "ccan/ccan/endian/endian.h" +#include +#include "internal.h" #ifdef __cplusplus extern "C" { @@ -83,6 +85,8 @@ size_t varint_to_bytes(uint64_t v, unsigned char *bytes_out); /* Read a variant from bytes */ size_t varint_from_bytes(const unsigned char *bytes, uint64_t *v); +WALLY_INTERNAL_API int scriptint_from_bytes(const unsigned char *bytes, size_t len, int64_t *value_out); + size_t scriptint_get_length(int64_t signed_v); size_t scriptint_to_bytes(int64_t signed_v, unsigned char *bytes_out); From 5f06c6d4a1923aede4f8d7c36c4c8107bb778b91 Mon Sep 17 00:00:00 2001 From: odudex Date: Fri, 4 Sep 2026 14:48:19 -0300 Subject: [PATCH 2/3] build: fix the mbed-tls option in CMake and the configure help text The CMake config template spelled the SHA-256 define as HAVE_MBEDTLS_SHA256_H_ (trailing underscore), so src/ccan_config.h never selected the mbedtls backend under CMake and silently built the software SHA-256 instead. The --enable-mbed-tls help string was a copy of the --enable-minimal one. --- _cmake/config.h.in | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/_cmake/config.h.in b/_cmake/config.h.in index fd045ba27..0a8f3b2a7 100644 --- a/_cmake/config.h.in +++ b/_cmake/config.h.in @@ -20,7 +20,7 @@ #cmakedefine HAVE_INLINE_ASM @HAVE_INLINE_ASM@ /* Define to 1 if you have the header file. */ -#cmakedefine HAVE_MBEDTLS_SHA256_H_ @HAVE_MBEDTLS_SHA256_H@ +#cmakedefine HAVE_MBEDTLS_SHA256_H @HAVE_MBEDTLS_SHA256_H@ /* Define to 1 if you have the header file. */ #cmakedefine HAVE_MBEDTLS_SHA512_H @HAVE_MBEDTLS_SHA512_H@ diff --git a/configure.ac b/configure.ac index c46255cda..84823b45f 100644 --- a/configure.ac +++ b/configure.ac @@ -83,7 +83,7 @@ AC_ARG_ENABLE(minimal, AS_HELP_STRING([--enable-minimal],[enable minimal size/memory footprint build (default: no)]), [minimal=$enableval], [minimal=no]) AC_ARG_ENABLE(mbed-tls, - AS_HELP_STRING([--enable-mbed-tls],[enable minimal size/memory footprint build (default: no)]), + AS_HELP_STRING([--enable-mbed-tls],[use mbed-tls hashing functions where available (default: no)]), [mbedtls=$enableval], [mbedtls=no]) AC_ARG_ENABLE(secp256k1-tests, AS_HELP_STRING([--enable-secp256k1-tests],[enable secp256k1 tests (default: no)]), From 5b08c090b2d0301d3674dc5d2bd8ea29c5dda829 Mon Sep 17 00:00:00 2001 From: odudex Date: Fri, 4 Sep 2026 14:52:03 -0300 Subject: [PATCH 3/3] hmac: derive the HMAC block size from the hash, not the context layout The HMAC implementation used sizeof() of a hash context member to obtain the block size and used that member as scratch space for the key block. That ties HMAC to the context layout of whichever hashing backend is selected, and is impossible for opaque backends such as PSA Crypto. Use the block size constant of the hash (64 for SHA-256, 128 for SHA-512) and a local key block instead. The byte operations are unchanged for all backends. --- src/hmac.c | 23 ++++------------------- src/hmac.inl | 33 +++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/hmac.c b/src/hmac.c index 4b68784d7..c57adcee2 100644 --- a/src/hmac.c +++ b/src/hmac.c @@ -3,6 +3,7 @@ #include #include #include +#include #ifdef SHA_T #undef SHA_T @@ -11,15 +12,7 @@ #define SHA_PRE(name) sha256 ## name #define HMAC_FUNCTION hmac_sha256_impl #define WALLY_HMAC_FUNCTION wally_hmac_sha256 -#ifdef CCAN_CRYPTO_SHA256_USE_MBEDTLS -#ifndef CONFIG_MBEDTLS_HARDWARE_SHA -#define SHA_CTX_BUFF c.MBEDTLS_PRIVATE(buffer) -#else -#define SHA_CTX_BUFF c.buffer -#endif -#else -#define SHA_CTX_BUFF buf.u8 -#endif +#define SHA_BLOCK_LEN 64 #include "hmac.inl" #undef SHA_T @@ -30,14 +23,6 @@ #define HMAC_FUNCTION hmac_sha512_impl #undef WALLY_HMAC_FUNCTION #define WALLY_HMAC_FUNCTION wally_hmac_sha512 -#undef SHA_CTX_BUFF -#ifdef CCAN_CRYPTO_SHA512_USE_MBEDTLS -#ifndef CONFIG_MBEDTLS_HARDWARE_SHA -#define SHA_CTX_BUFF c.MBEDTLS_PRIVATE(buffer) -#else -#define SHA_CTX_BUFF c.buffer -#endif -#else -#define SHA_CTX_BUFF buf.u8 -#endif +#undef SHA_BLOCK_LEN +#define SHA_BLOCK_LEN 128 #include "hmac.inl" diff --git a/src/hmac.inl b/src/hmac.inl index 00d4af912..68aa0a645 100644 --- a/src/hmac.inl +++ b/src/hmac.inl @@ -13,7 +13,7 @@ static void SHA_PRE(_mix)(struct SHA_T *sha, const unsigned char *pad, { struct SHA_PRE(_ctx) ctx; SHA_PRE(_init)(&ctx); - SHA_PRE(_update)(&ctx, pad, sizeof(ctx.SHA_CTX_BUFF)); + SHA_PRE(_update)(&ctx, pad, SHA_BLOCK_LEN); SHA_PRE(_update)(&ctx, data, data_len); SHA_PRE(_done)(&ctx, sha); wally_clear(&ctx, sizeof(ctx)); @@ -23,26 +23,31 @@ void HMAC_FUNCTION(struct SHA_T *sha, const unsigned char *key, size_t key_len, const unsigned char *msg, size_t msg_len) { - struct SHA_PRE(_ctx) ctx; - unsigned char ipad[sizeof(ctx.SHA_CTX_BUFF)]; - unsigned char opad[sizeof(ctx.SHA_CTX_BUFF)]; + /* The key block doubles as the buffer for the inner hash result */ + union { + unsigned char u8[SHA_BLOCK_LEN]; + struct SHA_T sha; + } key_block; + unsigned char ipad[SHA_BLOCK_LEN]; + unsigned char opad[SHA_BLOCK_LEN]; size_t i; - wally_clear(ctx.SHA_CTX_BUFF, sizeof(ctx.SHA_CTX_BUFF)); + BUILD_ASSERT(sizeof(struct SHA_T) <= SHA_BLOCK_LEN); + wally_clear(&key_block, sizeof(key_block)); - if (key_len <= sizeof(ctx.SHA_CTX_BUFF)) - memcpy(ctx.SHA_CTX_BUFF, key, key_len); + if (key_len <= SHA_BLOCK_LEN) + memcpy(key_block.u8, key, key_len); else - SHA_T((struct SHA_T *)ctx.SHA_CTX_BUFF, key, key_len); + SHA_T(&key_block.sha, key, key_len); - for (i = 0; i < sizeof(ctx.SHA_CTX_BUFF); ++i) { - opad[i] = ctx.SHA_CTX_BUFF[i] ^ 0x5c; - ipad[i] = ctx.SHA_CTX_BUFF[i] ^ 0x36; + for (i = 0; i < SHA_BLOCK_LEN; ++i) { + opad[i] = key_block.u8[i] ^ 0x5c; + ipad[i] = key_block.u8[i] ^ 0x36; } - SHA_PRE(_mix)((struct SHA_T *)ctx.SHA_CTX_BUFF, ipad, msg, msg_len); - SHA_PRE(_mix)(sha, opad, ctx.SHA_CTX_BUFF, sizeof(*sha)); - wally_clear_3(&ctx, sizeof(ctx), ipad, sizeof(ipad), opad, sizeof(opad)); + SHA_PRE(_mix)(&key_block.sha, ipad, msg, msg_len); + SHA_PRE(_mix)(sha, opad, key_block.u8, sizeof(*sha)); + wally_clear_3(&key_block, sizeof(key_block), ipad, sizeof(ipad), opad, sizeof(opad)); } int WALLY_HMAC_FUNCTION(const unsigned char *key, size_t key_len,