Unsigned integer wraparound in CCoinsViewCache::SpendCoin makes CCoinsViewCache::DynamicMemoryUsage() return bogus values #21001

1. 
   practicalswift commented at 9:00 pm on January 24, 2021: contributor

   When running the fuzz tests under -fsanitize=integer I stumbled upon this:

   0coins.cpp:114:22: runtime error: unsigned integer overflow: 0 - 96 cannot be represented in type 'unsigned long'
   

   https://github.com/bitcoin/bitcoin/blob/e31af1065e8a4a9dbb294e980047a135d4233962/src/coins.cpp#L111-L114

   Note that cachedCoinsUsage is decreased despite being zero which causes it to wrap around. This in turn makes CCoinsViewCache::DynamicMemoryUsage start returning bogus values.

   Nothing high priority of course, but perhaps worth fixing? :)

2. 
   sipa commented at 9:02 pm on January 24, 2021: member

   That would be a bug; it shouldn’t be possible to hit a situation where the dynamic memory counter goes negative. What input/sequence of operations triggers this?
3. 
   practicalswift commented at 9:56 pm on January 24, 2021: contributor

   I think the problem is in AddCoin: AFAICT cachedCoinsUsage is decreased also in the case when we throw std::logic_error("Attempted to overwrite an unspent coin (when possible_overwrite is false)").

   This sequence triggers the problem:

   1. A successful AddCoin(…, …, false) call
   2. A failed AddCoin(…, …, false) call (std::logic_error)
   3. A AddCoin(…, …, false) call

   Let me know if further details are needed.

4. 
   sipa commented at 9:22 am on January 25, 2021: member

   @practicalswift logic_error triggered indicates incorrect usage, and outside of tests is equivalent to an assertion failure; after that no guarantees are expected to hold.
5. 
   practicalswift commented at 11:49 am on January 25, 2021: contributor

   @sipa OK, thanks for clarifying. I’ll make the fuzzing harness stop at the first std::logic_error then :)
6. 
   danben commented at 6:16 pm on February 22, 2021: contributor

   Is there still anything to be done here or can this be closed?
7. MarcoFalke closed this on Feb 22, 2021

   ---

8. DrahtBot locked this on Aug 18, 2022

Contributors
practicalswift sipa danben