sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 12:02:48 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #17624: net: Fix an uninitialized read in ProcessMessage(…, "tx", …) when receiving a transaction we already have

Browse Source 
73b96c94cb6c2afdee7f151768a96944ecaf9d9b net: Fix uninitialized read in ProcessMessage(...) (practicalswift)

Pull request description:

  Fix an uninitialized read in `ProcessMessage(…, "tx", …)` when receiving a transaction we already have.

  The uninitialized value is read and used on [L2526 in the case of `AlreadyHave(inv) == true`](d8a66626d6/src/net_processing.cpp (L2494-L2526)).

  Proof of concept being run against a `bitcoind` built with MemorySanitizer (`-fsanitize=memory`):

  ```
  $ ./p2p-uninit-read-in-conditional-poc.py
  Usage: ./p2p-uninit-read-in-conditional-poc.py <dstaddr> <dstport> <net>
  $ bitcoind -regtest &
  $ ./p2p-uninit-read-in-conditional-poc.py 127.0.0.1 18444 regtest
  SUMMARY: MemorySanitizer: use-of-uninitialized-value
  [1]+  Exit 77                 bitcoind -regtest
  $
  ```

  Proof of concept being run against a `bitcoind` running under Valgrind (`valgrind --exit-on-first-error`):

  ```
  $ valgrind -q --exit-on-first-error=yes --error-exitcode=1 bitcoind -regtest &
  $ ./p2p-uninit-read-in-conditional-poc.py 127.0.0.1 18444 regtest
  ==27351== Conditional jump or move depends on uninitialised value(s)
  [1]+  Exit 1                  valgrind -q --exit-on-first-error=yes --error-exitcode=1 bitcoind -regtest
  $
  ```

  Proof of concept script:

  ```
  #!/usr/bin/env python3

  import sys

  from test_framework.mininode import NetworkThread
  from test_framework.mininode import P2PDataStore
  from test_framework.messages import CTransaction, CTxIn, CTxOut, msg_tx

  def send_duplicate_tx(dstaddr="127.0.0.1", dstport=18444, net="regtest"):
      network_thread = NetworkThread()
      network_thread.start()

      node = P2PDataStore()
      node.peer_connect(dstaddr=dstaddr, dstport=dstport, net=net)()
      node.wait_for_verack()

      tx = CTransaction()
      tx.vin.append(CTxIn())
      tx.vout.append(CTxOut())
      node.send_message(msg_tx(tx))
      node.send_message(msg_tx(tx))
      node.peer_disconnect()
      network_thread.close()

  if __name__ == "__main__":
      if len(sys.argv) != 4:
          print("Usage: {} <dstaddr> <dstport> <net>".format(sys.argv[0]))
          sys.exit(0)
      send_duplicate_tx(sys.argv[1], int(sys.argv[2]), sys.argv[3])
  ```

  Note that the transaction in the proof of concept is the simplest possible, but really any transaction can be used. It does not have to be a valid transaction.

  This bug was introduced in #15921 ("validation: Tidy up ValidationState interface") which was merged in to `master` 28 days ago.

  Luckily this bug was caught before being part of any Bitcoin Core release :)

ACKs for top commit:
  jnewbery:
    utACK 73b96c94cb6c2afdee7f151768a96944ecaf9d9b
  laanwj:
    ACK 73b96c94cb6c2afdee7f151768a96944ecaf9d9b, thanks for discovering and reporting this before it ended up in a release.

Tree-SHA512: 7ce6b8f260bcdd9b2ec4ff4b941a891bbef578acf4456df33b7a8d42b248237ec4949e65e2445b24851d1639b10681c701ad500b1c0b776ff050ef8c3812c795
This commit is contained in:
Wladimir J. van der Laan 2019-11-28 12:58:29 +01:00 committed by PastaPastaPasta
parent eec81f7b33
commit 27ecb07c8a
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/consensus/validation.h
View File

@ -103,7 +103,7 @@ inline ValidationState::~ValidationState() {};
class TxValidationState : public ValidationState {
private:
TxValidationResult m_result;
TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
public:
bool Invalid(TxValidationResult result,
const std::string &reject_reason="",
@ -118,7 +118,7 @@ public:
class BlockValidationState : public ValidationState {
private:
BlockValidationResult m_result;
BlockValidationResult m_result = BlockValidationResult::BLOCK_RESULT_UNSET;
public:
bool Invalid(BlockValidationResult result,
const std::string &reject_reason="",