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

Unlimited Token Approvals Not Revoked on Router Update

Summary

The StrategyOP contract fails to revoke token approvals when updating the router address, potentially leaving lingering unlimited approvals on old router addresses.

Description

The vulnerability stems from two design oversights:

  1. No explicit approval management during router updates

  2. Missing function to update router address safely

Vulnerable Code

Current Implementation in _initStrategy:

function _initStrategy() internal {
router = 0xa062aE8A9c5e11aaA026fc2670B0D65cCc8B2858;
underlying.safeApprove(address(router), type(uint256).max);
}

Here we approve type(uint256).max amount of underlying tokens to router which is hardcoded.
The setRouter() function is used to set another address as router by management in StategyOp.sol but it doesn't revoke the approval of tokens to 0 of past router address.

function setRouter(address _router) external onlyManagement {
router = _router;
underlying.safeApprove(router, type(uint256).max);
// @audit on setting router the approval should be removed from the old router
}

Impact

  1. Potential unauthorized access to underlying tokens

  2. Multiple routers having simultaneous unlimited approvals

  3. Risk of fund loss if old router is compromised

Recommendations

Implement proper router update function:

function setRouter(address _router) external onlyManagement {
// Remove approval from the old router
+ underlying.safeApprove(address(router), 0);
// Set the new router
router = ICurveRouterNG(_router);
// Approve the new router
underlying.safeApprove(_router, type(uint256).max);
}

Reference Findings:

https://solodit.cyfrin.io/issues/01-lingering-token-approvals-in-landmanager-may-lead-to-unauthorized-staking-of-munchables-code4rena-munchables-munchables-git

https://solodit.cyfrin.io/issues/l-08-no-way-to-revoke-approval-for-unsupported-assets-pashov-audit-group-none-ionprotocol-july-markdown

https://solodit.cyfrin.io/issues/l-05-no-way-to-revoke-approval-from-blacklisted-operators-pashov-audit-group-none-sofamon-august-markdown

https://solodit.cyfrin.io/issues/l-07-approvals-should-be-revoked-when-unstaking-in-honeylockersol-pashov-audit-group-none-interpol-markdown

Updates

Appeal created

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

Old router approval is not revoked after an update

inallhonesty Lead Judge 7 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.