0x52 - Hardcoded divider address in RollerUtils is incorrect and will brick autoroller

[sherlock-admin]

0x52

high

Hardcoded divider address in RollerUtils is incorrect and will brick autoroller

Summary

RollerUtils uses a hard-coded constant for the Divider. This address is incorrect and will cause a revert when trying to call AutoRoller#cooldown. If the adapter is combineRestricted then LPs could potentially be unable to withdraw or eject.

Vulnerability Detail

address internal constant DIVIDER = 0x09B10E45A912BcD4E80a8A3119f0cfCcad1e1f12;

RollerUtils uses a hardcoded constant DIVIDER to store the Divider address. There are two issues with this. The most pertinent issue is that the current address used is not the correct mainnet address. The second is that if the divider is upgraded, changing the address of the RollerUtils may be forgotten.

    (, uint48 prevIssuance, , , , , uint256 iscale, uint256 mscale, ) = DividerLike(DIVIDER).series(adapter, prevMaturity);

With an incorrect address the divider#series call will revert causing RollerUtils#getNewTargetedRate to revert, which is called in AutoRoller#cooldown. The result is that the AutoRoller cycle can never be completed. LP will be forced to either withdraw or eject to remove their liquidity. Withdraw only works to a certain point because the AutoRoller tries to keep the target ratio. After which the eject would be the only way for LPs to withdraw. During eject the AutoRoller attempts to combine the PT and YT. If the adapter is also combineRestricted then there is no longer any way for the LPs to withdraw, causing loss of their funds.

Impact

Incorrect hard-coded divider address will brick autorollers for all adapters and will cause loss of funds for combineRestricted adapters

Code Snippet

AutoRoller.sol#L853-L914

Tool used

Manual Review

Recommendation

RollerUtils DIVIDER should be set by constructor. Additionally RollerUtils should be deployed by the factory constructor to make sure they always have the same immutable divider reference.

[jparklev]

RollerUtils uses a hardcoded constant DIVIDER to store the Divider address. There are two issues with this. The most pertinent issue is that the current address used is not the correct mainnet address. The second is that if the divider is upgraded, changing the address of the RollerUtils may be forgotten.

We consider upgrading the divider a very disruptive and unlikely change that we'd need to redeploy most of our system for anyway, so we're not concerned with hardcoding as is. In addition, the address is for the goerli divider, so it was valid in the context we were testing it in.

That said, as we thought about it again, the divider address should actually be an immutable passed in via constructor, so we're validating this one and appreciate having a reason to rethink our assumptions

[jparklev]

Fix: sense-finance/auto-roller#17

[aktech297]

Changes looks fine.
As a suggestion, input address can be validated.