maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge #16572: wallet: Fix Char as Bool in Wallet

Browse source 
2dbfb37b40 Fix Char as Bool in interfaces (Jeremy Rubin)

Pull request description:

  In a few places in src/wallet/wallet.h, we use a char when semantically we want a bool.

  This is kind of an issue because it means we can unserialize the same transaction with different fFromMe flags (as differing chars) and evaluate the following section in wallet/wallet.cpp
  ```c++
          if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe)
           {
               wtx.fFromMe = wtxIn.fFromMe;
               fUpdated = true;
           }
  ```
  incorrectly (triggering an fUpdated where both fFromMe values represent true, via different chars).

  I don't think this is a vulnerability, but it's just a little messy and unsemantic, and could lead to issues with stored wtxIns not being findable in a map by their hash.

  The serialize/unserialize code for bool internally uses a char, so it should be safe to make this substitution.

  NOTE: Technically, this is a behavior change -- I haven't checked too closely that nowhere is depending on storing information in this char. Theoretically, this could break something because after this change a tx unserialized with such a char would preserve it's value, but now it is converted to a ~true~ canonical bool.

ACKs for top commit:
  achow101:
    Code review ACK 2dbfb37b40
  meshcollider:
    Code review ACK 2dbfb37b40

Tree-SHA512: 8c0dc9cf672aa2276c694facbf50febe7456eaa8bf2bd2504f81a61052264b8b30cdb5326e1936893adc3d33504667aee3c7e207a194c71d87b3e7b5fe199c9d
This commit is contained in:
fanquake 2019-08-21 15:06:21 +08:00
parent 8ee572f094 2dbfb37b40
commit 01ebaa05a4
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
1 changed files with 5 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/wallet/wallet.h
View file

@ -444,7 +444,7 @@ public:
* on this bitcoin node, and set to 0 for transactions that were created
* externally and came in through the network or sendrawtransaction RPC.
*/
char fFromMe;
bool fFromMe;
int64_t nOrderPos; //!< position in ordered transaction list
std::multimap<int64_t, CWalletTx*>::const_iterator m_it_wtxOrdered;
@ -501,8 +501,8 @@ public:
std::vector<char> dummy_vector1; //!< Used to be vMerkleBranch
std::vector<char> dummy_vector2; //!< Used to be vtxPrev
char dummy_char = false; //!< Used to be fSpent
s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_char;
bool dummy_bool = false; //!< Used to be fSpent
s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_bool;
}
template<typename Stream>
@ -512,8 +512,8 @@ public:
std::vector<uint256> dummy_vector1; //!< Used to be vMerkleBranch
std::vector<CMerkleTx> dummy_vector2; //!< Used to be vtxPrev
char dummy_char; //! Used to be fSpent
s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_char;
bool dummy_bool; //! Used to be fSpent
s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_bool;
ReadOrderPos(nOrderPos, mapValue);
nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0;