net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix

This commit is contained in:
Sebastian Falbesoner 2020-04-28 19:19:34 +02:00
parent 6e3fc7492a
commit 1ad8ea2b73
4 changed files with 5 additions and 34 deletions

View file

@ -31,8 +31,6 @@ CBloomFilter::CBloomFilter(const unsigned int nElements, const double nFPRate, c
* Again, we ignore filter parameters which will create a bloom filter with more hash functions than the protocol limits * Again, we ignore filter parameters which will create a bloom filter with more hash functions than the protocol limits
* See https://en.wikipedia.org/wiki/Bloom_filter for an explanation of these formulas * See https://en.wikipedia.org/wiki/Bloom_filter for an explanation of these formulas
*/ */
isFull(false),
isEmpty(true),
nHashFuncs(std::min((unsigned int)(vData.size() * 8 / nElements * LN2), MAX_HASH_FUNCS)), nHashFuncs(std::min((unsigned int)(vData.size() * 8 / nElements * LN2), MAX_HASH_FUNCS)),
nTweak(nTweakIn), nTweak(nTweakIn),
nFlags(nFlagsIn) nFlags(nFlagsIn)
@ -47,7 +45,7 @@ inline unsigned int CBloomFilter::Hash(unsigned int nHashNum, const std::vector<
void CBloomFilter::insert(const std::vector<unsigned char>& vKey) void CBloomFilter::insert(const std::vector<unsigned char>& vKey)
{ {
if (isFull) if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700)
return; return;
for (unsigned int i = 0; i < nHashFuncs; i++) for (unsigned int i = 0; i < nHashFuncs; i++)
{ {
@ -55,7 +53,6 @@ void CBloomFilter::insert(const std::vector<unsigned char>& vKey)
// Sets bit nIndex of vData // Sets bit nIndex of vData
vData[nIndex >> 3] |= (1 << (7 & nIndex)); vData[nIndex >> 3] |= (1 << (7 & nIndex));
} }
isEmpty = false;
} }
void CBloomFilter::insert(const COutPoint& outpoint) void CBloomFilter::insert(const COutPoint& outpoint)
@ -74,10 +71,8 @@ void CBloomFilter::insert(const uint256& hash)
bool CBloomFilter::contains(const std::vector<unsigned char>& vKey) const bool CBloomFilter::contains(const std::vector<unsigned char>& vKey) const
{ {
if (isFull) if (vData.empty()) // Avoid divide-by-zero (CVE-2013-5700)
return true; return true;
if (isEmpty)
return false;
for (unsigned int i = 0; i < nHashFuncs; i++) for (unsigned int i = 0; i < nHashFuncs; i++)
{ {
unsigned int nIndex = Hash(i, vKey); unsigned int nIndex = Hash(i, vKey);
@ -112,10 +107,8 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx)
bool fFound = false; bool fFound = false;
// Match if the filter contains the hash of tx // Match if the filter contains the hash of tx
// for finding tx when they appear in a block // for finding tx when they appear in a block
if (isFull) if (vData.empty()) // zero-size = "match-all" filter
return true; return true;
if (isEmpty)
return false;
const uint256& hash = tx.GetHash(); const uint256& hash = tx.GetHash();
if (contains(hash)) if (contains(hash))
fFound = true; fFound = true;
@ -177,19 +170,6 @@ bool CBloomFilter::IsRelevantAndUpdate(const CTransaction& tx)
return false; return false;
} }
void CBloomFilter::UpdateEmptyFull()
{
bool full = true;
bool empty = true;
for (unsigned int i = 0; i < vData.size(); i++)
{
full &= vData[i] == 0xff;
empty &= vData[i] == 0;
}
isFull = full;
isEmpty = empty;
}
CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const double fpRate) CRollingBloomFilter::CRollingBloomFilter(const unsigned int nElements, const double fpRate)
{ {
double logFpRate = log(fpRate); double logFpRate = log(fpRate);

View file

@ -45,8 +45,6 @@ class CBloomFilter
{ {
private: private:
std::vector<unsigned char> vData; std::vector<unsigned char> vData;
bool isFull;
bool isEmpty;
unsigned int nHashFuncs; unsigned int nHashFuncs;
unsigned int nTweak; unsigned int nTweak;
unsigned char nFlags; unsigned char nFlags;
@ -64,7 +62,7 @@ public:
* nFlags should be one of the BLOOM_UPDATE_* enums (not _MASK) * nFlags should be one of the BLOOM_UPDATE_* enums (not _MASK)
*/ */
CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn); CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn);
CBloomFilter() : isFull(true), isEmpty(false), nHashFuncs(0), nTweak(0), nFlags(0) {} CBloomFilter() : nHashFuncs(0), nTweak(0), nFlags(0) {}
ADD_SERIALIZE_METHODS; ADD_SERIALIZE_METHODS;
@ -90,9 +88,6 @@ public:
//! Also adds any outputs which match the filter to the filter (to match their spending txes) //! Also adds any outputs which match the filter to the filter (to match their spending txes)
bool IsRelevantAndUpdate(const CTransaction& tx); bool IsRelevantAndUpdate(const CTransaction& tx);
//! Checks for empty and full filters to avoid wasting cpu
void UpdateEmptyFull();
}; };
/** /**

View file

@ -3193,7 +3193,6 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
{ {
LOCK(pfrom->m_tx_relay->cs_filter); LOCK(pfrom->m_tx_relay->cs_filter);
pfrom->m_tx_relay->pfilter.reset(new CBloomFilter(filter)); pfrom->m_tx_relay->pfilter.reset(new CBloomFilter(filter));
pfrom->m_tx_relay->pfilter->UpdateEmptyFull();
pfrom->m_tx_relay->fRelayTxes = true; pfrom->m_tx_relay->fRelayTxes = true;
} }
return true; return true;

View file

@ -25,7 +25,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
fuzzed_data_provider.ConsumeIntegral<unsigned int>(), fuzzed_data_provider.ConsumeIntegral<unsigned int>(),
static_cast<unsigned char>(fuzzed_data_provider.PickValueInArray({BLOOM_UPDATE_NONE, BLOOM_UPDATE_ALL, BLOOM_UPDATE_P2PUBKEY_ONLY, BLOOM_UPDATE_MASK}))}; static_cast<unsigned char>(fuzzed_data_provider.PickValueInArray({BLOOM_UPDATE_NONE, BLOOM_UPDATE_ALL, BLOOM_UPDATE_P2PUBKEY_ONLY, BLOOM_UPDATE_MASK}))};
while (fuzzed_data_provider.remaining_bytes() > 0) { while (fuzzed_data_provider.remaining_bytes() > 0) {
switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 4)) { switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 3)) {
case 0: { case 0: {
const std::vector<unsigned char> b = ConsumeRandomLengthByteVector(fuzzed_data_provider); const std::vector<unsigned char> b = ConsumeRandomLengthByteVector(fuzzed_data_provider);
(void)bloom_filter.contains(b); (void)bloom_filter.contains(b);
@ -65,9 +65,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
(void)bloom_filter.IsRelevantAndUpdate(tx); (void)bloom_filter.IsRelevantAndUpdate(tx);
break; break;
} }
case 4:
bloom_filter.UpdateEmptyFull();
break;
} }
(void)bloom_filter.IsWithinSizeConstraints(); (void)bloom_filter.IsWithinSizeConstraints();
} }