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

balanceOf() accounting is generally manipulatable or griefable and should be avoided

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));
}
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.