Description
claimAndSwap()
in the strategies relies solely on balanceOf() accounting for their balances, which can be manipulated or griefed.
Vulnerable Code
claimAndSwap
:
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));
}
Impact
balanceOf() accounting is generally prone to manipulation and gas griefing attacks and should be avoided as much as possible.
Severity: Low (Griefing)
Recommended Fix
Use the return values of the swap functions to protect against slippage:
StrategyOp::_swapUnderlyingAsset
:
- function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal {
+ function _swapUnderlyingToAsset(uint256 _amount, uint256 minOut, IVeloRouter.route[] calldata _path) internal returns(uint256){
require(minOut > _amount, "minOut too low");
uint256 underlyingBalance = underlying.balanceOf(address(this));
require(underlyingBalance >= _amount, "not enough underlying balance");
- IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
+ uint256[] memory values = IVeloRouter(router).swapExactTokensForTokens(_amount, minOut, _path, address(this), block.timestamp);
+ return values[0];
}
StrategyOp::claimAndSwap
:
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path)
external
onlyKeepers
{
transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
- _swapUnderlyingToAsset(_amountClaim, _minOut, _path);
+ uint256 amountActualOut = _swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
- require((balAfter - balBefore) >= _minOut, "Slippage too high");
+ require(amountActualOut >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}