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

1 files changed, 1 insertions, 0 deletions

diff --git a/src/util/overflow.h b/src/util/overflow.h
index 5982af8d04..c70a8b663e 100644
--- a/src/util/overflow.h
+++ b/src/util/overflow.h
@@ -6,6 +6,7 @@
 #define BITCOIN_UTIL_OVERFLOW_H
 
 #include <limits>
+#include <optional>
 #include <type_traits>
 
 template <class T>