DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Invalid

Lack of Validation on Swap Paths in `claimAndSwap`

Summary

The claimAndSwap function allows the keeper role to specify arbitrary swap paths when swapping underlying tokens to asset tokens. Without validation, a malicious keeper can provide a swap path that diverts funds through attacker-controlled tokens or contracts, leading to significant financial losses for the strategy.

Vulnerability Details

In the claimAndSwap function, the keeper specifies the swap route via the _path parameter:

function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
_swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

Key issues:

  • No Validation of _path:

    • The contract does not check whether the swap path ends with the expected asset token.

    • It does not verify if the intermediate tokens in _path are safe or approved.

  • Potential for Malicious Swap Paths:

    • A malicious keeper can craft a swap path that routes through tokens or contracts they control.

    • This can result in funds being diverted away from the strategy.

Impact

  • Complete Loss of Swapped Funds:

    • Funds intended to be converted from underlying to asset can be siphoned off during the swap.

  • Security Breach:

    • The strategy's integrity is compromised if a keeper can manipulate funds without checks.

  • Loss of Trust:

    • Investors may lose confidence in the strategy due to vulnerabilities in fund management.

Proof of Concept (POC)

  1. Setup:

    • The keeper has the onlyKeepers role, allowing them to call claimAndSwap.

  2. Crafting Malicious Swap Path:

    • The keeper defines a swap path that includes a malicious token or contract (MaliciousToken).

    • For example, the path could be: WETH -> MaliciousToken -> WETH.

  3. Execution:

    • The keeper calls claimAndSwap with the malicious path.

IRamsesRouter.route[] memory maliciousPath = new IRamsesRouter.route[]();
maliciousPath[0] = IRamsesRouter.route({
from: address(underlying),
to: address(maliciousToken),
stable: false
});
maliciousPath[1] = IRamsesRouter.route({
from: address(maliciousToken),
to: address(asset),
stable: false
});
strategyArb.claimAndSwap(_amountClaim, _minOut, maliciousPath);
  1. Malicious Contract Drains Funds:

    • The MaliciousToken contract has a transfer function that transfers tokens to the attacker's address whenever it is called.

  2. Result:

    • During the swap, the underlying tokens are routed through MaliciousToken, which steals the tokens.

    • The strategy contract ends up with fewer asset tokens than expected, or none at all.

Recommendations

  • Validate Swap Paths:

    • Ensure that the swap path starts with underlying and ends with asset.

    • Verify that all intermediate tokens and pairs are approved and safe.

function validateSwapPath(IRamsesRouter.route[] calldata _path) internal view returns (bool) {
// Check that the path starts with 'underlying'
if (_path[0].from != address(underlying)) return false;
// Check that the path ends with 'asset'
if (_path[_path.length - 1].to != address(asset)) return false;
// Additional checks on intermediate tokens if necessary
return true;
}
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
require(validateSwapPath(_path), "Invalid swap path");
// Proceed with function
}
Updates

Appeal created

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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