net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix
Summary:
The BIP37 bloom filter class CBloomFilter contains two flags isEmpty/isFull together with an update method with the purpose to, according to the comments, "avoid wasting cpu", i.e. the mechanism should serve as an optimization for the trivial cases of empty (all bits zero) or full (all bits one) filters. However, the real reason of adding those flags (introduced with commit 37c6389 by gmaxwell) was a covert fix of CVE-2013-5700, a vulnerability that allowed a divide-by-zero remote node crash. According to gmaxwell himself (#9060 (comment)): the IsEmpty/IsFull optimizations were largely a pretextual optimization intended to make unexploitable a remote crash vulnerability (integer division by zero) that existed in the original bloom filtering code without disclosing it. I'm doubtful that they are all that useful. :) For more information on how to trigger this crash, see PR #18515 which contains a detailled description and a regression test. It has also been discussed on a recent PR club meeting on fuzzing. The covert fix code already led to issues and PR based on the wrong assumption that the flags are there for optimization reasons (see #16886 and #16922). This PR gets rid of the flags and the update method and just focuses on the CVE fix itself, i.e. it can be seen as a revert of the covert fix commit modulo the actual fix.
Backport of core PR18806.
Test Plan:
ninja all check-all ninja bitcoin-fuzzers ./src/test/fuzz/bloom_filter <path_to_corpus>
Reviewers: #bitcoin_abc, PiRK
Reviewed By: #bitcoin_abc, PiRK
Differential Revision: https://reviews.bitcoinabc.org/D9051