DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: low
Valid

setRouter function will revert if the router is changed and the protocol wants to go back to a previous rotuer

Summary

The protocol will never be able to go back to a previously used router (Ramses router) if they ever update the router address.

Vulnerability Details

In both the Arbitrum and Optimism chain contracts, the setRouterfunction uses safeApproveincorrectly. It can be seen that the code tries to make the allowance for the routers to type(uint256).max, this will fail if there is already an non-zero allowance set for that address(router).

For example:

  • The initial router address is A which gives an approval of type(uint256).maxto the Aaddress.

  • Now assume that the ramses router is upgraded to address B, so the protocol team changes the address to Bby calling the setRouter function which gives address B an approval of type(uint256).max.

  • Now if the protocol ever wants to go back to the previous router address A(maybe because there is a vulnerability in the new router or some other reason). They will never be able to do that. Since safeApproverequired a 0 allowance for the address.
    Thus calling the setRouterfunction will revert and the protocol can never go back to the old router address. Furthermore even while moving to a new router, the approval for the old router still remains which can be exploited if the old router had turned malicious.
    https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L42-L45

function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max);
}



Impact

The protocol will not be able to set the desired router address because of this bug. Which could cause a downtime in the protocol or in more extreme cases a loss of funds (if the old router had turned malicious).

Note: Ramses Exchange is planning to have many future upgraded (according to their docs to V2 and V3).

Tools Used

Manual Review

Recommendations

It is better to make the approval of the old router 0 while upgrading to a new one. This will prevent a revert when setting the new router and handles the case of a malicious old router.
Update the code as following

function setRouter(address _router) external onlyManagement {
underlying.safeApprive(router,0); // added code to make allowance 0
router = _router;
underlying.safeApprove(router, type(uint256).max);
}
Updates

Appeal created

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Old router approval is not revoked after an update

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Old router approval is not revoked after an update

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.