script: reject malformed script integers, add tests

_cmake/config.h.in

@@ -20,7 +20,7 @@
 #cmakedefine HAVE_INLINE_ASM @HAVE_INLINE_ASM@
 
 /* Define to 1 if you have the <mbedtls/sha256.h,> 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 <mbedtls/sha512.h> header file. */
 #cmakedefine HAVE_MBEDTLS_SHA512_H @HAVE_MBEDTLS_SHA512_H@

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)]),

src/Makefile.am

@@ -224,28 +224,29 @@ 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)
 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)
 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,23 +265,31 @@ 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)
 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)

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} $<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/src>)
+#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)

src/ctest/test_script.c

@@ -0,0 +1,88 @@
+#include "config.h"
+
+#include <wally_core.h>
+#include "script_int.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdbool.h>
+
+#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;
+}

src/hmac.c

@@ -3,6 +3,7 @@
 #include <ccan/ccan/crypto/sha256/sha256.h>
 #include <ccan/ccan/crypto/sha512/sha512.h>
 #include <include/wally_crypto.h>
+#include <ccan/ccan/build_assert/build_assert.h>
 
 #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"

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,

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;
 }
 

src/script.h

@@ -2,6 +2,8 @@
 #define LIBWALLY_CORE_SCRIPT_INTERNAL_H 1
 
 #include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
 
 /* Get the size of a push from the script push opcode(s) */
 int script_get_push_size_from_bytes(

src/script_int.h

@@ -2,6 +2,8 @@
 #define LIBWALLY_CORE_SCRIPT_INT_H 1
 
 #include "ccan/ccan/endian/endian.h"
+#include <string.h>
+#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);