Transaction deserialisation allows an invalid input to aggsig::verify_single

[hashmap]

Fuzz test found the following issue (on the current master as well as on my validation refactoring branch):

[libsecp256k1] illegal argument: !secp256k1_fe_is_zero(&ge->x) [5/1940]
==30456== ERROR: libFuzzer: deadly signal
#0 0x5625d6ab9947 in __sanitizer_print_stack_trace /checkout/src/libcompiler_builtins/compiler-rt/lib/asan/asan_stack.cc:38:3
#1 0x5625d6b0a35f in fuzzer::Fuzzer::CrashCallback() /home/arch/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/86bcaac/libfuzzer/FuzzerLoop.cpp:233:3
8
#2 0x5625d6b0a219 in fuzzer::Fuzzer::StaticCrashSignalCallback() /home/arch/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/86bcaac/libfuzzer/FuzzerLoop.cpp:206:19
#3 0x5625d6af708b in fuzzer::CrashHandler(int, siginfo_t*, void*) /home/arch/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/86bcaac/libfuzzer/FuzzerUtilPosix.cpp:36:36
#4 0x7f9be9f543af (/usr/lib/libpthread.so.0+0x123af)
#5 0x7f9be9d99b5e in __GI_raise (/usr/lib/libc.so.6+0x37b5e)
#6 0x7f9be9d84451 in __GI_abort (/usr/lib/libc.so.6+0x22451)
#7 0x5625d6c99858 in default_illegal_callback_fn /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/depend/secp256k1-zkp/src/secp256k1.c:52:5
#8 0x5625d6c7d37a in secp256k1_callback_call /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/depend/secp256k1-zkp/src/util.h:24:5
#9 0x5625d6c99ac1 in secp256k1_pubkey_load /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/depend/secp256k1-zkp/src/secp256k1.c:162:5
#10 0x5625d6c99cf8 in secp256k1_ec_pubkey_serialize /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/depend/secp256k1-zkp/src/secp256k1.c:209:9
#11 0x5625d6c9f8f0 in secp256k1_compute_sighash_single /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/depend/secp256k1-zkp/src/modules/aggsig/main_impl.h:46:5
#12 0x5625d6c9fc21 in secp256k1_aggsig_verify_single /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/depend/secp256k1-zkp/src/modules/aggsig/main_impl.h:538:9
#13 0x5625d6c79ebb in secp256k1zkp::aggsig::verify_single::h40553dddea324fd9 /home/arch/.cargo/git/checkouts/rust-secp256k1-zkp-2d57249f422a426c/2564e8c/src/aggsig.rs:117:8
#14 0x5625d6b4a7d3 in grin_core::core::transaction::TxKernel::verify::h63afa9a2574dd182 /home/arch/grin/core/src/core/transaction.rs:205:6
#15 0x5625d6b4f0ee in grin_core::core::transaction::Transaction::verify_kernel_signatures::h1a4c33f469da938e /home/arch/grin/core/src/core/transaction.rs:418:3
#16 0x5625d6b5292f in grin_core::core::transaction::Transaction::validate::h70622b8581694f0e /home/arch/grin/core/src/core/transaction.rs:458:2
#17 0x5625d6b4e55c in $LT$grin_core..core..transaction..Transaction$u20$as$u20$grin_core..ser..Readable$GT$::read::hd9d9fd213f120a06 /home/arch/grin/core/src/core/transaction.rs:317:2
#18 0x5625d6a1bdeb in grin_core::ser::deserialize::h2d752af19c0e7375 /home/arch/grin/core/src/ser.rs:247:1
#19 0x5625d6a18014 in rust_fuzzer_test_input /home/arch/grin/core/fuzz/fuzz_targets/transaction_read.rs:11:56
#20 0x5625d6ad305d in libfuzzer_sys::test_input_wrap::$u7b$$u7b$closure$u7d$$u7d$::h8462e073cd794c30 /home/arch/.cargo/git/checkouts/libfuzzer-sys-e07fde05820d7bc6/86bcaac/src/lib.rs:11:8
#21 0x5625d6aed7ab in std::panicking::try::do_call::ha8622a6a12f180ba /checkout/src/libstd/panicking.rs:310:39
#22 0x5625d6ea2418 in __rust_maybe_catch_panic /checkout/src/libpanic_abort/lib.rs:39:4

[hashmap]

Here is a file which crashes tx read here: https://github.com/mimblewimble/grin/blob/master/core/fuzz/fuzz_targets/transaction_read.rs
(it's a binary file zipped to calm down github)

tx-read-crash.zip

[garyyu]

How to feed this data into the Fuzz test? to reproduce the crash issue.

[hashmap]

@garyyu I'm afraid you need to write a unit test which read the input and calls Read method. Don't see how run single input in fuzz test itself.

[garyyu]

Deserialize your rx-read-crash data and get this:

        kernels: [
            TxKernel {
                features: DEFAULT_KERNEL,
                fee: 2,
                lock_height: 0,
                excess: Commitment(085908ab7472bbc712e802af4876fd309b1e8ef7d4c86d8540ecced705f59fd0fb),
                excess_sig: Signature(
                    190c28924b836ce8d8c51298d9894f0d541251089d4bf88d13f75fabf6d600000000000000000000000000000000000000000000000000000000000000000000
                )
            }
        ]

The signature contains 32bytes 0 for r part which will be taken as a public key, so this is the root cause: secp256k1 lib don't have protection for 0 public key.

I will give fix in rust-secp256k1-zkp repo.

And in Grin repo, I can add a test case to cover this fuzz finding. Thanks @hashmap

[hashmap]

Oh, nice, fuzzer seems to be updated, now it prints out an offensive input:

MS: 0 ; base unit: 0000000000000000000000000000000000000000
0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x1,0x0,0x25,0x0,0x0,0x0,0x0,0x0,0x0,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x8d,0x0,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x19,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x65,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,
\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00%\x00\x00\x00\x00\x00\x00eeeeeeeeeeeeeeeeeeeeeeeee\x8d\x00eeeeeeeeeeeeee\x19eeeeeeeeeeeeeeeeeeeee\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00

Base64: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAJQAAAAAAAGVlZWVlZWVlZWVlZWVlZWVlZWVlZWVlZWWNAGVlZWVlZWVlZWVlZWVlGWVlZWVlZWVlZWVlZWVlZWVlZWVlZQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=

[garyyu]

This one can be re-tested to confirm no more such kind of problem.
It was fixed by mimblewimble/rust-secp256k1-zkp#24 and #1381

[hashmap]

@garyyu tested, no issues. Good work!