Open code423n4 opened 2 years ago
jkilpatr
commented
2 years ago This is frankly a good point. We need to be able to skip nonces because we may sometimes create batches that are not profitable or validator sets that we don't want to bother submitting.
A reasonable mitigation for this issue would be to limit how far ahead we can skip at any one time. Preventing error or hostile action from locking funds in the bridge forever by setting the maximum nonce.
albertchon
commented
2 years ago Well this actually isn't an accurate attack since the nonce representation on the module side is uint64 whereas it's a uint256 on the cosmos side.
Still, I think this attack is quite hard to trigger naturally since the nonce is incremented by 1 on the module side and as sending 2^64 -1 transactions on a Cosmos chain (to trigger this overflow) would be cost prohibitive/
And I think attacks that assume validators collude aren't really attacks since that's the natural trust assumption already.
Marking this one as the primary.
loudoguno
commented
2 years ago reopening as per judges assessment as "primary issue" on findings sheet
Handle
pauliax
Vulnerability details
Impact
I am concerned why invalidationId, invalidationNonce or valsetNonce are only required to be greater than the previous value. Why did you choose this approach instead of just simply asking for an incremented value? While this may not be a problem if the validators are honest, but otherwise, they may submit a nonce of MAX UINT and thus block the whole system as it would be no longer possible to submit a greater value. Again, just wanted you to be aware of this issue, not sure how likely this to happen is in practice, it depends on the honesty of validators so you better know.
Recommended Mitigation Steps
I didn't receive an answer on Discord so decided to submit this FYI to decide if that's a hazard or no.