This is a follow-up to commit e26b62093ae21e89ed7d36a24a6b863f38ec631d with the following fixes:
- Fix unsigned integer overflow in
ignore(), whennReadPoswraps. - Fix unsigned integer overflow in
read(), whennReadPoswraps. - Fix read-past-the-end in
read(), whennReadPoswraps.
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().
0diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
1index 922fd8e513..ec6ea93919 100644
2--- a/src/test/coins_tests.cpp
3+++ b/src/test/coins_tests.cpp
4@@ -534,6 +534,20 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization)
5 } catch (const std::ios_base::failure&) {
6 }
7
8+ CDataStream foo{0, 0};
9+ auto size{std::numeric_limits<uint32_t>::max()};
10+ foo.resize(size);
11+ BOOST_CHECK_EQUAL(foo.size(), size);
12+ foo.ignore(std::numeric_limits<int32_t>::max());
13+ size -= std::numeric_limits<int32_t>::max();
14+ BOOST_CHECK_EQUAL(foo.size(), size);
15+ foo.ignore(std::numeric_limits<int32_t>::max());
16+ size -= std::numeric_limits<int32_t>::max();
17+ BOOST_CHECK_EQUAL(foo.size(), size);
18+ BOOST_CHECK_EQUAL(foo.size(), 1);
19+ foo.ignore(7); // Should overflow, as the size is only 1
20+ BOOST_CHECK_EQUAL(foo.size(), uint32_t(1 - 7));
21+
22 // Very large scriptPubKey (3*10^9 bytes) past the end of the stream
23 CDataStream tmp(SER_DISK, CLIENT_VERSION);
24 uint64_t x = 3000000000ULL;
