Skip to content

Conversation

practicalswift
Copy link
Contributor

Avoid signed integer overflow when loading a mempool.dat file with a malformed time field.

Avoid the following signed integer overflow:

$ xxd -p -r > mempool.dat-crash-1 <<EOF
0100000000000000000000000004000000000000000000000000ffffffff
ffffff7f00000000000000000000000000
EOF
$ cp mempool.dat-crash-1 ~/.bitcoin/regtest/mempool.dat
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
validation.cpp:5079:23: runtime error: signed integer overflow: 9223372036854775807 + 1209600 cannot be represented in type 'long'
    #0 0x5618d335197f in LoadMempool(CTxMemPool&) src/validation.cpp:5079:23
    #1 0x5618d3350df3 in CChainState::LoadMempool(ArgsManager const&) src/validation.cpp:4217:9
    #2 0x5618d2b9345f in ThreadImport(ChainstateManager&, std::vector<boost::filesystem::path, std::allocator<boost::filesystem::path> >, ArgsManager const&) src/init.cpp:762:33
    #3 0x5618d2b92162 in AppInitMain(util::Ref const&, NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_14::operator()() const src/init.cpp:1881:9

This PR was broken out from PR #20089. Hopefully this PR is trivial to review.

Fixes a subset of #19278.

@maflcko
Copy link
Member

maflcko commented Nov 11, 2020

review ACK ee11a41

@Crypt-iQ
Copy link
Contributor

crACK ee11a41

@maflcko maflcko merged commit 027e51f into bitcoin:master Nov 12, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 12, 2020
…pool.dat file with a malformed time field

ee11a41 Avoid signed integer overflow when loading a mempool.dat file with a malformed time field (practicalswift)

Pull request description:

  Avoid signed integer overflow when loading a `mempool.dat` file with a malformed time field.

  Avoid the following signed integer overflow:

  ```
  $ xxd -p -r > mempool.dat-crash-1 <<EOF
  0100000000000000000000000004000000000000000000000000ffffffff
  ffffff7f00000000000000000000000000
  EOF
  $ cp mempool.dat-crash-1 ~/.bitcoin/regtest/mempool.dat
  $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
  validation.cpp:5079:23: runtime error: signed integer overflow: 9223372036854775807 + 1209600 cannot be represented in type 'long'
      #0 0x5618d335197f in LoadMempool(CTxMemPool&) src/validation.cpp:5079:23
      #1 0x5618d3350df3 in CChainState::LoadMempool(ArgsManager const&) src/validation.cpp:4217:9
      #2 0x5618d2b9345f in ThreadImport(ChainstateManager&, std::vector<boost::filesystem::path, std::allocator<boost::filesystem::path> >, ArgsManager const&) src/init.cpp:762:33
      #3 0x5618d2b92162 in AppInitMain(util::Ref const&, NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_14::operator()() const src/init.cpp:1881:9
  ```

  This PR was broken out from PR bitcoin#20089. Hopefully this PR is trivial to review.

  Fixes a subset of bitcoin#19278.

ACKs for top commit:
  MarcoFalke:
    review ACK ee11a41
  Crypt-iQ:
    crACK ee11a41

Tree-SHA512: 227ab95cd7d22f62f3191693b455eacfa8e36534961bee12c622fc9090957cfb29992eabafa74d806a336e03385aa8f98b7ce734f04b0b400e33aa187d353337
@practicalswift practicalswift deleted the load-mempool-time-integer-overflow branch April 10, 2021 19:42
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 23, 2021
…malformed time field

Summary: This is a backport of [[bitcoin/bitcoin#20372 | core#20372]]

Test Plan:
This causes an error before applying the commit, and it works after the change:
```
$ cmake .. -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SANITIZERS=undefined
$ xxd -p -r > mempool.dat-crash-1 <<EOF
0100000000000000000000000004000000000000000000000000ffffffff
ffffff7f00000000000000000000000000
EOF
$ cp mempool.dat-crash-1 /bitcoinddata/regtest/mempool.dat
$ ninja
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
```

```
../src/validation.cpp:5853:23: runtime error: signed integer overflow: 9223372036854775807 + 1209600 cannot be represented in type 'long'
    #0 0x55f14103ffcd in LoadMempool(Config const&, CTxMemPool&) /home/pierre/dev/bitcoin-abc/build_ubsan/../src/validation.cpp:5853:23
    #1 0x55f14103fb65 in CChainState::LoadMempool(Config const&, ArgsManager const&) /home/pierre/dev/bitcoin-abc/build_ubsan/../src/validation.cpp:4821:9
...
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10725
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants