-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure Spec mitigates Double Spending by Colliding InternalH #738
Comments
In other words, CoinCommitment() isn't computationally binding, and the above attack is a consequence of that. |
Yes, this appears to work. Excellent catch!
Or φ. |
Awesome find @defuse! I was also suspicious of the security of that function but didn't realize there was an attack at the time. |
In the proof of Balance in section D.3, the attack violates either Condition I, if the double-spend is within a single Pour (i.e. the doubled coin is spent in both inputs), or Condition II, if the double-spend occurs across Pours. For Condition I, the failure is here:
For Condition II, the failure is here:
(Note that the proofs should really be distinguishing between COMMr and COMMs since they are different constructions, but we'll let that pass for now.) Let's look at whether the above arguments actually follow from the definition of binding for a commitment scheme (spoiler: they do). Note that the Zerocash paper doesn't define binding at all; it only says:
Finding a precisely-stated definition of computational binding security was suprisingly time-consuming. Wikipedia gives a silly definition that I won't confuse you with. (Exercise: beside being asymptotic, what else is wrong with it?) Definition 24.1 in http://www.zurich.ibm.com/~cca/sft13/smart08chaps2426.pdf will suffice:
(r is the randomness, x is the committed value.) This indeed would be sufficient for the arguments in the security proof. COMMr would have this binding property if Leading128 ∘ CRH were collision-resistant, but obviously, that doesn't hold at a 128-bit security level. (Actually, we need that the composition of COMMr and COMMs is binding on all of the inputs, i.e. apk, ρ, and v, but the problem in this attack is with COMMr.) Note that collision resistance of Leading128 ∘ CRH was not explicitly assumed in the construction of COMM in section 5.1. (Even if we didn't need that, we would still need Leading253 ∘ CRH to be collision-resistant, which is also not explicitly assumed, because of the truncation of hSig and ρ.) |
So let's see: we need to commit to apk, ρ, and v, which are a total of 256+256+64 = 576 bits. It's questionable whether we actually need the statistical hiding property, or whether we can get it (note that the scheme must also be noninteractive, which rules out some constructions in the literature) at reasonable circuit cost. Note that we don't have "everlasting anonymity" anyway because of the encryption. If we only need computational hiding and computational binding, then we could just do: InternalH = CRH(apk || ρ) |
Another option is to use the full SHA-256 hash: cm = SHA-256(apk || v || ρ || r) (Length extension attacks aren't a problem here.) This also requires two compression function evaluations but leaves more scope for possible future changes, because we're effectively saving 256 bits from the input to the second compression function (minus the minimum one byte of padding) by putting it in the chaining variable. Edit: swap order of v and ρ to be the same order as in the definition of a coin and as encoded in a coin plaintext. |
Using two hashes or full SHA256 would be nicer because we could have better domain separation between things, and not have to worry as much about things like this: #500 (comment) |
I'm inclined to use the full hash for this. It isn't very much more complicated or expensive to implement the full hash in the circuit. That leaves the question of whether we're also going to change the PRFs to use the full hash. Either way, there's still a domain separation issue: if we don't change the PRFs, then the IV will be the same for a use of the bare compression function and a use of the full hash (i.e. the intermediate chaining variable between the two blocks of the full hash will be the same as the output of some use of the bare compression function). This is fixable if we put the domain separation tag at the beginning. Changing the PRFs to use the full hash means that we only have 504 bits of input available rather than 512, due to the padding. This is not necessarily a problem; we could truncate ask and φ to 244 bits (4 bits of domain separation, 8 bits padding) without significant loss of security, since those values do not need collision resistance. [Edit: this was wrong, I'd forgotten the 64-bit length field. So we have 440 bits of input available, and would have had to truncate ask and φ by a further 64 bits to 180 bits, which would have been dubious.] If the Merkle tree sticks with the SHA-256 compression function, then there seems little motivation to change the PRF to use the full hash (we would have to do the analysis assuming that the compression function is used in multiple places, anyway). If we change the Merkle tree to something else then I'd like to get rid of the only remaining use of the bare compression function for the PRF. |
(Strictly speaking, full SHA-256 takes a bit string input and so we could hash 511 bits in a single compression function evaluation. Maybe we should do that; most SHA-256 implementations do not support bit string inputs but that's not really a problem.) [Edit: 447 bits, not 511 bits.] |
Instead of SHA-256, could SHA-512/256 be used? :) |
@paragonie-scott Thanks for the suggestion, but we don't have a zk-SNARK circuit implementation of SHA-512, and I suspect it has similar circuit complexity to SHA-256 with two blocks of input, in any case. |
This is fixed in the current spec (not in the current implementation). |
There's a variation of the attack that I overlooked, where you collide InternalH by varying apk, without necessarily changing ρ. (It's sufficient to know both corresponding ask; the serial numbers / nullifiers will be different because ask is the key input to PRFsn/nf.) Our new note commitment scheme blocks that variation as well. |
I think I found an attack that would let an attacker create as much money as they want for themselves at the cost of finding 128-bit hash collisions.
Background: A coin is a tuple (apk, v, ρ, r). The coin commitment CoinCommitment((apk, v, ρ, r)) is computed as:
The truncation of InternalH is done to make the commitment statistically-hiding, i.e no matter how much computational resources I have, I can't find out what v or apk are.
Attack: Because InternalH is only 128 bits, in 2^64 operations I can find some ρ != ρ' such that:
And therefore:
To carry out a double-spend attack I first acquire some real Zcash (e.g. by buying it with USD), and then double spend it to myself as follows. Ignoring the faerie gold fix, I create a new coin for myself, but as I do so I make sure to find a collision (apk, v, ρ, r) and (apk, v, ρ', r) where ρ != ρ'. By completeness of the protocol, I'll be able to spend the (apk, v, ρ, r) coin, and so that's what I do. This will reveal the serial number PRFsnask(ρ). After that first spend has made it on to the ledger, as far as I can see, nothing prevents me from spending that coin again, using ρ', because:
After the faerie gold fix, I'm forced to find colliding ρ by altering hsig, and I'm forced to commit to either ρ or ρ' because hsig gets published. This doesn't prevent the attack, since when I go to spend for the second time it is not going to be noticed that ρ' doesn't match the old hsig, at least not according to the current protocol.
Fix: Maybe CoinCommitment needs to be collision-resistant? Or maybe we should re-use hsig as a commitment to ρ and check it when I try to spend with ρ?
The text was updated successfully, but these errors were encountered: