aboutsummaryrefslogtreecommitdiff
path: root/src/txrequest.cpp
diff options
context:
space:
mode:
authorMarcoFalke <falke.marco@gmail.com>2022-02-21 08:09:02 +0100
committerMarcoFalke <falke.marco@gmail.com>2022-02-21 08:09:18 +0100
commitabaf943477ada2c78ed99b5208287d9959a43769 (patch)
tree2c83033908311cf502514a2052ef95920b3b65c2 /src/txrequest.cpp
parent3de5fcc94fcdf3b2b597399809c92065c1997fcf (diff)
parentfa1b89a6bdbab50bdb0504782afd4bb3375d1b57 (diff)
downloadbitcoin-abaf943477ada2c78ed99b5208287d9959a43769.tar.xz
Merge bitcoin/bitcoin#24231: streams: Fix read-past-the-end and integer overflows
fa1b89a6bdbab50bdb0504782afd4bb3375d1b57 scripted-diff: Rename nReadPos to m_read_pos in streams.h (MarcoFalke) fa56c79df91e5d87533af38b64f4f4148a48a276 Make CDataStream work properly on 64-bit systems (MarcoFalke) fab02f799194c75af7def3a2ab45c443b75de230 streams: Fix read-past-the-end and integer overflows (MarcoFalke) Pull request description: This is a follow-up to commit e26b62093ae21e89ed7d36a24a6b863f38ec631d with the following fixes: * Fix unsigned integer overflow in `ignore()`, when `nReadPos` wraps. * Fix unsigned integer overflow in `read()`, when `nReadPos` wraps. * Fix read-past-the-end in `read()`, when `nReadPos` wraps. This shouldn't be remote-exploitable, because it requires a stream of more than 1GB of size. However, it might be exploitable if the attacker controls the datadir (I haven't checked). A unit test for the overflow in `ignore()` looks like following. It is left as an excercise to the reader to replace `foo.ignore(7)` with the appropriate call to `read()` to reproduce the overflow and read-error in `read()`. ```diff diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 922fd8e513..ec6ea93919 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -534,6 +534,20 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization) } catch (const std::ios_base::failure&) { } + CDataStream foo{0, 0}; + auto size{std::numeric_limits<uint32_t>::max()}; + foo.resize(size); + BOOST_CHECK_EQUAL(foo.size(), size); + foo.ignore(std::numeric_limits<int32_t>::max()); + size -= std::numeric_limits<int32_t>::max(); + BOOST_CHECK_EQUAL(foo.size(), size); + foo.ignore(std::numeric_limits<int32_t>::max()); + size -= std::numeric_limits<int32_t>::max(); + BOOST_CHECK_EQUAL(foo.size(), size); + BOOST_CHECK_EQUAL(foo.size(), 1); + foo.ignore(7); // Should overflow, as the size is only 1 + BOOST_CHECK_EQUAL(foo.size(), uint32_t(1 - 7)); + // Very large scriptPubKey (3*10^9 bytes) past the end of the stream CDataStream tmp(SER_DISK, CLIENT_VERSION); uint64_t x = 3000000000ULL; ``` ACKs for top commit: klementtan: Code Review ACK fa1b89a6bdbab50bdb0504782afd4bb3375d1b57: Tree-SHA512: 67f0a1baafe88eaf1dc844ac55b638d5cf168a18c945e3bf7a2cb03c9a5976674a8e3af2487d8a2c3eae21e5c0e7a519c8b16ee7f104934442e2769d100660e9
Diffstat (limited to 'src/txrequest.cpp')
0 files changed, 0 insertions, 0 deletions