3

I've been trying to follow the Bitcoin Unlimited hack timeline (2017-Mar) and there seems to be conflicting stories on both sides. It's obviously to confuse the 3rd party into finding the truth. My main questions are:

  • When was the exploit discovered and by whom?
  • When was the fix issued after the discovery of the exploit?
  • When did the hack occur to knock out over 700+ Bitcoin Unlimited nodes?
  • There were reports of these 700+ nodes going offline and then within a few hours, it all came back online. If this is the case, how does one conclude that these 700+ nodes are controlled by a few people?
  • Who are the main characters on both sides in this hack to affect news stories and perception?
  • Where was the hack in the code exactly?

1 Answer 1

4

The commit implementing the fix was issued Mar 14, 2017, 3:16 PM GMT. A Github release was made at 7:39 PM GMT. It was announced on Reddit at 7:59 PM GMT. The vulnerability was discovered sometime before the git commit, but it's unclear how long before.

According to nodecounter, things were fine at about 6 PM GMT, when there were 776 nodes. That rapidly dropped, with 696 nodes at 7 PM GMT. It bottomed out at 11 PM, with 182 nodes. At 9 AM GMT the following day, things were mostly back to normal, with 626 nodes. I don't think it's very accurate to say that BU reacted abnormally quickly, or that 100% of nodes came back.

Where was the hack in the code exactly?

There were two unrelated issues. Both issues were in how Bitcoin Unlimited responds to network packets asking for thinblocks.

  1. Reachable assert.

    BU had this code:

    void SendXThinBlock(CBlock &block, CNode* pfrom, const CInv &inv)
    {
        if (inv.type == MSG_XTHINBLOCK)
        {
            // code
        }
        else if (inv.type == MSG_THINBLOCK)
        {
            // code code code
        }
        else
        {
            assert(0);
        }
        // more code
    }
    

    (I've removed irrelevant parts to make it issue easier to see.)

    This means,

    • If the request is a thinblock, reply in this way,
    • if the request is an xthinblock, reply in that way,
    • if it's neither, crash.

    Why does it do that? I assume that the person who wrote this code assumed that this function would only be called for one of those two types of requests. This function is called in two places. In the first place where this function is called, that assumption is correct. In the second place, it is wrong. This bug was introduced May 2016 in this commit.

    The fix is to DoS ban the node that sent the request, rather than crashing.

  2. Unchecked return value

    BU had this code:

    BlockMap::iterator mi = mapBlockIndex.find(inv.hash);
    // code code
    if (!ReadBlockFromDisk(block, (*mi).second, consensusParams))
        assert(!"cannot load block from disk");
    

    The problem here is that if inv.hash does not exist in mapBlockIndex, the first line will return mapBlockIndex.end(). The next line will then pass a null pointer to ReadBlockFromDisk. Then, the node will crash. This bug was introduced Jan 2016 in this commit.

3
  • How many developers reviewed this part of the code? It's always looks simple after the fact to spot this. But as a non-code reviewer, was this a hard bug to spot? Mar 16, 2017 at 15:06
  • @duckx I don't think either bug was obvious without the benefit of hindsight. SendXThinBlock was correct at the time it was written, and only became vulnerable after a second path to it was created.
    – Nick ODell
    Mar 16, 2017 at 17:46
  • @duckx The second was a find() that was not followed by a check for whether it was in the map. But Bitcoin does this in other places, and it's perfectly OK: github.com/bitcoin/bitcoin/blob/… Is there an obvious way to distinguish these two cases? I'm not sure.
    – Nick ODell
    Mar 16, 2017 at 17:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.