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

Improper Allowance Management for Previous Router Poses Security Risks

Summary

The strategy contract allows management to change the router via the setRouter function. While the new router receives maximum approval for WETH, the approval for the previous router is not revoked. This oversight can result in security risks, particularly if the previous router becomes malicious or compromised.

Vulnerability Details

The router acts as a DEX through which the keeper swaps WETH for ALETH in case of depegging:

address public router;
function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IRamsesRouter.route[] calldata _path) internal {
// TODO : we swap WETH to ALETH -> need to check that price is better than 1:1
// uint256 oraclePrice = 1e18 * 101 / 100;
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
IRamsesRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
}

Hence, the router requires maximum approval to execute these swaps. The functions _initStrategy and setRouter are responsible for granting this approval:

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

Also, the strategy is likely to have underlyingBalance in the contract.

function balanceDeployed() public view returns (uint256) {
return transmuter.getUnexchangedBalance(address(this)) + underlying.balanceOf(address(this)) + asset.balanceOf(address(this));
}

However, the approval for the previous router is not revoked when setRouter is called. As a result, the previous router retains unlimited access to the contract’s WETH balance, even though it is no longer intended for use. This could have severe consequences if:

  1. The previous router becomes malicious.

  2. The previous router is compromised in an exploit or contract upgrade vulnerability, as seen in incidents like the LQDX hack (reference).

Impact

Retaining maximum approval for the previous router exposes the contract’s WETH to theft or abuse, which could result in significant financial losses. This risk is exacerbated by the likelihood of a router being compromised or maliciously upgraded.

Tools Used

Manual

Recommendations

To mitigate the issue, before assigning maximum approval to the new router, revoke the approval for the old router to prevent unauthorized access.

Updates

Appeal created

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

Old router approval is not revoked after an update

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