search

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

1 stars 0 forks source link

cumulativePower check should be inclusive #26

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

Based on my understanding cumulativePower checks should be inclusive to indicate when the threshold is met. Otherwise, there might be impossible to reach it in certain cases (e.g. when 100% power is required). Replace '>' with '>=' in constructor and function checkValidatorSignatures: if (cumulativePower > _powerThreshold) { break; } require( cumulativePower > _powerThreshold, "Submitted validator set signatures do not have enough power." );

Recommended Mitigation Steps

cumulativePower >= _powerThreshold

jkilpatr commented 2 years ago

I would classify this as low risk since the bridge would never in any sane situation be configured to require 100% of the power. It's a valid report in the context that a slightly more permissive check could save the day in very specific situations.

loudoguno commented 2 years ago

reopening as per judges assessment as "primary issue" on findings sheet