code-423n4 / 2021-08-gravitybridge-findings

1 stars 0 forks source link

Validations of validators #30

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

Consider introducing upper and lower limits for the size of the validators array. Currently, there are no restrictions on validators array. In theory, a scenario exists where there are too many validators in the array with low individual powers and a high threshold so the loop that iterates over them will never finish (exceeds block gas limit). I advise setting boundaries for the size of validators (e.g. MIN_VALIDATORS and MAX_VALIDATORS). Every chain has its gas limit so you can individually test the capacity and then set it via the constructor. The lower limit could be set to 1 so that it wouldn't be possible to have 0 validators. It could also check that addresses are unique, no the same validator is present twice.

Recommended Mitigation Steps

Introduce validations for the array of validators.

jkilpatr commented 2 years ago

I had not considered this issue. From some rough napkin math a validator set update with 125 validators costs 706,459 gas, assuming linear scaling and given a block gas limit of 30 million that's ~6000 validators. Even if they cut the block gas limit by ~80% we're still looking at an enormous gas margin versus normal cosmos validator set sizes.

Good report, but I believe this a wontfix

albertchon commented 2 years ago

Cosmos-SDK based chains typically have < 200 validators so not a huge issue