Isn't this merkle method supposed to include the total number of BTC with the root hash? Otherwise user verification isn't even a valid proof of liabilities. So assuming it was done right, what was the total number of BTC kraken have, according to the root hash?
They made the decision that an auditor was to be the one to check assets against the liability sum. Since you can't see their asset sum, their liability sum is not useful to you; only your inclusion in that number (and the elimination of other foul play) matters. That means their trees and verifier need only hash your sum, not feed it up the tree as all five other implementations do.
However, olalonde and I decided to look more closely at what freedoms the existing code might give them...
Let's assume the auditor checked a fresh copy of the code out of GitHub and built it himself on a known-good machine (we have to assume, because unfortunately he doesn't say).
Fix:state explicitly that you checked hashes, built from source and ran on your own machine.
They've openly made a choice to
include accounts with negative balances. As discussed in the
issue I've raised, I think this is a bad idea. To summarise the points:
- genuine negative balances may often never be repaid, just as a customer who walks out of a bar after getting too much change from a 50 may never come back, so negative balances don't reduce your liabilities
- negative balances may be used for foul play, and it should be made as hard as possible for foul play to go undetected, so negative balances must not become a normal thing (much better to hear from your exchange "our assets only cover 99.8% of our liabilities" than enable cheating)
- users can't distinguish genuine negative balances from real ones, so including them gives no useful information
So while their tool appears to list negative balances explicitly while building the tree from accounts data, we'll also have to assume (because unfortunately, he doesn't say) that any listed negative balances were justified by Kraken and proven to be real users expected to make good, not fake accounts inserted to falsely lower liabilities.
Fix: disallow negative liabilities, as
described,
specified in a draft standard and
implemented elsewhere. Alternatively, explicitly state what the sum of negative liabilities was and that you were given satisfactory proof that those accounts belonged to real people.
No unique user-chosen information was included in the Merkle tree
leaf hash. That means their tool could group all users with the same balance, give them all the same nonce (which they call the "Submission code" in their
documentation), and convince them all that a single entry in the tree represented every one of them. The subversion possible this way depends on how many customers have identical balances, or balances close enough to not notice. Because the exchange can deliver nonces after the audit, this is possible even if the auditor is running the exact code published and it's working as expected. Fix:
inclusion of unique user-chosen info (such as e-mail address) in the hash, as
described,
specified in a draft standard and
implemented elsewhere. (If it's a requirement that auditors don't see this, hash it with the nonce first.)
Edit: linkify "leaf hash"Edit: add "Because the exchange can deliver nonces after the audit,"