diff --git a/src/common/script.c b/src/common/script.c index d85d05db..c0af557a 100644 --- a/src/common/script.c +++ b/src/common/script.c @@ -123,6 +123,12 @@ int format_opscript_script(const uint8_t script[], return -1; } + if (script_len > 83) { + // a script that is more than 83 bytes violates the "max 80 bytes total data" rule + // (+ 3 bytes of opcodes) and is therefore not standard in Bitcoin Core. + return -1; + } + strncpy(out, "OP_RETURN ", MAX_OPRETURN_OUTPUT_DESC_SIZE); int out_ctr = 10; @@ -150,21 +156,43 @@ int format_opscript_script(const uint8_t script[], if (opcode == OP_0) { out[out_ctr++] = '0'; } else if (opcode >= 1 && opcode <= 75) { + // opcodes between 1 and 75 indicate a data push of the corresponding length hex_length = opcode; - if (offset + hex_length > script_len) return -1; // out of bounds } else if (opcode == OP_PUSHDATA1) { - if (offset >= script_len) return -1; // out of bounds for length byte + // the next byte is the length + if (offset >= script_len) { + return -1; // out of bounds for length byte + } hex_length = script[offset++]; - if (hex_length > 80 || offset + hex_length > script_len) return -1; + if (hex_length <= 75) { + return -1; // non-standard, should have used the minimal push opcode + } } else if (opcode == OP_1NEGATE) { out[out_ctr++] = '-'; out[out_ctr++] = '1'; } else if (opcode >= OP_1 && opcode <= OP_16) { uint8_t num = opcode - 0x50; + // num is a number between 1 and 16 (included) if (num >= 10) { - out[out_ctr++] = '0' + (num / 10); + out[out_ctr++] = '1'; + num -= 10; + } + out[out_ctr++] = '0' + num; + } else { + // any other opcode is invalid or unsupported + return -1; + } + + if (offset + hex_length > script_len) { + // overflow, not enough bytes to read in the script + return -1; + } + + if (hex_length == 1) { + if (script[offset] == 0x81 || script[offset] <= 16) { + // non-standard, it should use OP_1NEGATE, or one of OP_0, ..., OP_16 + return -1; } - out[out_ctr++] = '0' + (num % 10); } if (hex_length > 0) { @@ -182,6 +210,12 @@ int format_opscript_script(const uint8_t script[], out[out_ctr++] = ' '; } + if (offset < script_len) { + // if there are still more opcodes left, we do not support this script + // (for example: more than 5 push opcodes) + return -1; + } + out[out_ctr - 1] = '\0'; return out_ctr; } diff --git a/src/common/script.h b/src/common/script.h index 1be90e90..42bab4f8 100644 --- a/src/common/script.h +++ b/src/common/script.h @@ -208,21 +208,27 @@ int get_script_address(const uint8_t script[], size_t script_len, char *out, siz /** * Formats a valid OP_RETURN script for user verification. The resulting string is "OP_RETURN - * ", where is written according to the rules below. Only scripts with a single push - * opcode are supported, and OP_PUSHDATA2 and OP_PUSHDATA4 are not supported. OP_1NEGATE is + * ", where is written according to the rules below. Only scripts with up to 5 push + * opcodes are supported, and OP_PUSHDATA2 and OP_PUSHDATA4 are not supported. OP_1NEGATE is * represented as "-1", and OP_0, OP_1, ..., OP_16 are represented in decimal ("0", "1", ..., "16"). * For other push opcodes, the data is represented in hexadecimal, two characters per byte, with the * "0x" prefix. * + * As a best-effort measure, this function returns an error if the transaction is non-standard + * according to the default rules of Bitcoin Core (maximum 80 bytes of data, only push + * instructions). Such transactions, even if valid, would not easily be relayed in the default + * mempool. + * An exception is an output script with a single OP_RETURN is accepted despite being non-standard, + * as such an output is used in BIP-0322. + * * The string is written onto `out` and is 0-terminated. Its length is returned. * * @param script the script to parse and format. * @param script_len the length of the script. - * @param out the output array, that must be at least MAX_OPRETURN_OUTPUT_DESC_SIZE bytes long. The - * longest possible string is "OP_RETURN 0x" followed by 160 hexadecimal characters, plus the - * terminating null character, for a total of 173 characters. + * @param out the output array, that must be at least MAX_OPRETURN_OUTPUT_DESC_SIZE bytes long. * @return The length of the string written into `out` (including the terminating 0) on success; -1 - * on error. + * on error, or if such an output would make the transaction be non-standard per the default relay + * rules of Bitcoin Core. */ int format_opscript_script(const uint8_t script[], size_t script_len, diff --git a/unit-tests/test_script.c b/unit-tests/test_script.c index ad445d85..213dde97 100644 --- a/unit-tests/test_script.c +++ b/unit-tests/test_script.c @@ -218,9 +218,6 @@ static void test_format_opscript_script_valid(void **state) { "0x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425" "262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a"); - uint8_t input20[] = {OP_RETURN, OP_PUSHDATA1, 7, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}; - CHECK_VALID_TESTCASE(input20, "OP_RETURN 0x01020304050607"); - uint8_t input21[] = {OP_RETURN, OP_PUSHDATA1, 80, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, @@ -239,10 +236,9 @@ static void test_format_opscript_script_valid(void **state) { uint8_t input23[] = {OP_RETURN, OP_0, OP_1, OP_5, OP_7, OP_16}; CHECK_VALID_TESTCASE(input23, "OP_RETURN 0 1 5 7 16"); - uint8_t input24[] = { - OP_RETURN, OP_8, OP_1NEGATE, OP_PUSHDATA1, 15, 1, 2, 3, 4, 5, 6, - 7, 8, 9, 10, 11, 12, 13, 14, 15, OP_0, OP_PUSHDATA1, - 7, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77}; + uint8_t input24[] = {OP_RETURN, OP_8, OP_1NEGATE, 15, 1, 2, 3, 4, 5, 6, + 7, 8, 9, 10, 11, 12, 13, 14, 15, OP_0, + 7, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77}; CHECK_VALID_TESTCASE(input24, "OP_RETURN 8 -1 0x0102030405060708090a0b0c0d0e0f 0 0x11223344556677"); @@ -275,6 +271,43 @@ static void test_format_opscript_script_invalid(void **state) { uint8_t input_extra_push[] = {OP_RETURN, 4, 1, 2, 3, 4, 42}; CHECK_INVALID_TESTCASE(input_extra_push); + + uint8_t input_6pushes[] = {OP_RETURN, OP_1, OP_2, OP_3, OP_4, OP_5, OP_6}; + CHECK_INVALID_TESTCASE(input_6pushes); + + // clang-format off + uint8_t input_too_long[] = { + OP_RETURN, + OP_PUSHDATA1, 81, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + 81 + }; + // clang-format on + CHECK_INVALID_TESTCASE(input_too_long); + + // not minimal push encodings + uint8_t input_pushdata_nonstandard[] = + {OP_RETURN, OP_PUSHDATA1, 7, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07}; + CHECK_INVALID_TESTCASE(input_pushdata_nonstandard); + + uint8_t input_negative1_notminimal_1[] = {OP_RETURN, 1, 0x81}; + CHECK_INVALID_TESTCASE(input_negative1_notminimal_1); + uint8_t input_negative1_notminimal_2[] = {OP_RETURN, OP_PUSHDATA1, 1, 0x81}; + CHECK_INVALID_TESTCASE(input_negative1_notminimal_2); + for (uint8_t i = 0; i <= 16; i++) { + uint8_t input_negative1_notminimal_push_1[] = {OP_RETURN, 1, i}; + CHECK_INVALID_TESTCASE(input_negative1_notminimal_push_1); + uint8_t input_negative1_notminimal_push_2[] = {OP_RETURN, OP_PUSHDATA1, 1, i}; + CHECK_INVALID_TESTCASE(input_negative1_notminimal_push_2); + } + + // transaction containing non-push opcodes are not standard + uint8_t input_non_push_opcode[] = {OP_RETURN, OP_3, OP_2, OP_ADD, OP_0}; + CHECK_INVALID_TESTCASE(input_non_push_opcode); } int main() {