Lack of wraparound protection in circuits
Description
In the join
, join_split
, and split
circuits, it is checked that the sum of the amounts of the input notes is equal to the sum of the amounts of the output notes, such as seen in this snippet from join_split
:
assert(in_amount_1 + in_amount_2 == out_amount_1 + out_amount_2);
This check is done without protecting against overflows, so equality is only checked to hold modulo the characteristic p
of the finite field that the witness variables are elements of. Thus, this check will pass, for example, for in_amount_1 = 0
, in_amount_2 = 0
, out_amount_1 = a
, and out_amount_2 = p - a
, for arbitrary a
. An attacker can thus create themselves notes with arbitrary amounts.
This is possible with both the join_split
and split
circuits. In the case of the join
circuit, overflows can only happen on the input side (as there is only one output note), hence a wraparound would be to the attacker's disadvantage (losing p
of the asset).
Impact
An attacker can create notes with arbitrary amounts at no cost, thereby allowing them to drain the asset vaults of all ERC-20 and ETH.
Recommendations
Ensure in the circuits that no overflow happens on addition. As only two summands are added, one way to do this would be to check that each summand lies in the range 0, ..., (p-1) / 2
.
Remediation
This issue has been acknowledged by Singularity, and a fix was implemented in commit 77df136c↗.