search

Gearbox-protocol / core-v3

Other
28 stars 4 forks source link

feat: add validation to `PoolQuotaKeeperV3.setGauge` #265

Closed lekhovitsky closed 1 month ago

lekhovitsky commented 1 month ago

Fixes https://github.com/spearbit-audits/review-gearbox/issues/32

StErMi commented 1 month ago

@lekhovitsky I think that it could be beneficial to revert the PoolQuotaKeeperV3.setGauge function if quotaTokensSet.length() != IRateKeeper(gauge).tokenSetLength() (note that this function does not exist yet in IRateKeeper implementations).

While it's true that the current implementation of GaugeV3, TumblerV3 and PoolQuotaKeeper do not allow having a length mismatch, it's something that could be missed in future implementations. When a new GaugeV3/TumblerV3 contract is deployed with an existing (already configured) PoolQuotaKeeper, the deployer must cover all the already quoted token existing in PoolQuotaKeeper. If the token is already quoted (like in this case), the call to IPoolQuotaKeeperV3(poolQuotaKeeper).addQuotaToken(...) is correctly skipped (otherwise it would revert). If the token has not been quoted yet, PKQ is called.

This means that even if the GaugeV3 or TumblerV3 is freshly deployed and configured to match an existing PQK, it will be impossible to have (with the current implementation), a IRateKeeper contract that has more tokens configured than the related PoolQuotaKeeper.

As I said, my suggestion is something that would be needed to make the PoolQuotaKeeper and the protocol in general more bulletproof for the future.

lekhovitsky commented 1 month ago

@StErMi Acknowledged, but since this kind of misconfiguration can't really lead to system blocking or incorrect behavior right now, it's probably not worth dealing with at this stage.