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

Unlimited Approval Exploit in `StrategyMainnet`, `StrategyArb`, and `StrategyOp` Contracts

Summary

The StrategyMainnet, StrategyArb, and StrategyOp contracts contain a vulnerability related to unlimited token approvals. Specifically, the strategies grant unlimited approval to their respective router contracts, allowing them to transfer any amount of tokens from the strategies. If a router is malicious or compromised, it can drain all funds deposited in the strategies.

The exploit is successfully demonstrated using a malicious router that extracts all strategy funds by leveraging the unlimited approval. This issue arises from the lack of restriction in token approval and inadequate handling of trust assumptions in the claimAndSwap and _deployFunds functions.

Vulnerability Details

The root cause lies in the contracts granting unlimited approval of tokens to their respective routers during the initialization or interaction processes (_initStrategy, _deployFunds, and claimAndSwap). This practice assumes the router is always trustworthy, but if compromised, it can perform unauthorized transfers.

Key Steps of the Attack:

  1. Unlimited Approval: The strategy pre-approves an unlimited amount of tokens to the router. This occurs in the _initStrategy function:

    underlying.safeApprove(address(router), type(uint256).max);

This approval remains active indefinitely, even if the router is replaced or compromised.

  1. Malicious Router: The router is replaced by a malicious contract that abuses the approval to transfer tokens:

    function exchange(...) external returns (uint256) {
    token.transferFrom(msg.sender, address(this), amountIn + 1);
    }

    The malicious router can extract more tokens than intended by the strategy.

  2. Execution: When claimAndSwap is called, the malicious router uses the unlimited approval to drain all strategy funds instead of performing a legitimate swap.

  3. Result: The strategy’s balance becomes zero, and all tokens are transferred to the malicious router.

Impact

  • Loss of Funds: An attacker controlling the router can drain all tokens approved to it, resulting in the complete loss of strategy funds.

  • Reputation Damage: Exploiting this vulnerability can undermine trust in the protocol and its security practices.

  • Systemic Risk: If multiple strategies share similar logic, the vulnerability may propagate across the system, amplifying the impact.

Proof of Concept

Exploit Code

The following test demonstrates the exploit by replacing the router with a malicious one. The malicious router drains all funds from the strategy:

function test_Exploit() public {
console.log("=== Starting Exploit Test ===");
// User deposits 1000 tokens into the strategy
console.log("User deposits 1000 tokens into the strategy...");
vm.prank(user);
strategy.deposit(1000e18);
// Confirm strategy holds the deposited tokens
uint256 stratBalance = token.balanceOf(address(strategy));
console.log("Strategy balance after deposit:", stratBalance);
assertEq(stratBalance, 1000e18, "Strategy should have 1000 tokens");
// Execute claimAndSwap, triggering the malicious router to drain funds
console.log("Calling claimAndSwap with amountIn = 1000 tokens...");
vm.prank(user);
strategy.claimAndSwap(1000e18, 1001e18);
// Check post-exploit balances
uint256 stratBalanceAfter = token.balanceOf(address(strategy));
uint256 routerBalanceAfter = token.balanceOf(address(maliciousRouter));
console.log("Strategy balance after exploit:", stratBalanceAfter);
console.log("Malicious router balance after exploit:", routerBalanceAfter);
assertEq(stratBalanceAfter, 0, "Strategy should have 0 tokens after exploit");
assertEq(
routerBalanceAfter,
1000e18,
"Malicious router should have stolen 1000 tokens"
);
console.log("=== Exploit Test Completed Successfully ===");
}

Exploit Logs

Logs:
=== Starting Exploit Test ===
User deposits 1000 tokens into the strategy...
Strategy balance after deposit: 1000000000000000000000
Calling claimAndSwap with amountIn = 1000 tokens...
Strategy balance after exploit: 0
Malicious router balance after exploit: 1000000000000000000000
=== Exploit Test Completed Successfully ===

This confirms that the malicious router successfully drains all funds from the strategy.

Tools Used

  • Foundry: Used to develop and run tests to validate the vulnerability.

  • Manual Code Review: Confirmed the issue exists across all three contracts.

Recommendations

To mitigate this vulnerability, implement on-demand token approvals:

  1. Approve Only When Needed:
    Replace unlimited approvals with exact approvals just before the operation:

    asset.safeApprove(address(router), 0); // Reset approval
    asset.safeApprove(address(router), amountIn); // Approve exact amount
  2. Reset Approvals Post-Operation:
    Reset approvals to zero immediately after completing the operation:

    asset.safeApprove(address(router), 0);

Mitigated Code Example

The following mitigated code implements these recommendations:

function claimAndSwap(
uint256 _amountClaim,
uint256 _minOut,
uint256 _routeNumber
) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
require(_minOut > _amountClaim, "minOut too low");
// Approve only the exact amount for the router
underlying.safeApprove(address(router), 0);
underlying.safeApprove(address(router), _amountClaim);
router.exchange(
routes[_routeNumber],
swapParams[_routeNumber],
_amountClaim,
_minOut,
pools[_routeNumber],
address(this)
);
// Reset approval after the operation
underlying.safeApprove(address(router), 0);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
}

Test Results After Mitigation

The following test verifies that the mitigation is effective. It ensures the router fails to exploit the strategy:

function testMitigation() public {
console.log("=== Testing Mitigation ===");
vm.prank(user);
strategy.deposit(1000e18);
uint256 initialAssets = strategy.totalAssets();
console.log("Initial strategy totalAssets:", initialAssets);
assertEq(initialAssets, 1000e18, "Should have 1000 after deposit");
vm.expectRevert(); // Expect the exploit to fail
strategy.claimAndSwap(500e18, 501e18, 0);
console.log(
"The exploit did not succeed, approvals are limited. Test passed."
);
}

Test Logs After Mitigation

Logs:
=== Testing Mitigation ===
Initial strategy totalAssets: 1000000000000000000000
The exploit did not succeed, approvals are limited. Test passed.

This confirms the exploit is no longer viable after applying the mitigation.

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

Support

FAQs

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