Description:
StrategyOp::claimAndSwap, StrategyArb::claimAndSwap and StrategyMainnet::claimAndSwap are lacking checks for _amountClaim which can result in keepers being able to acquire funds not meant for their strategy, but other keepers strategies.
Vulnerable Code:
StrategyOp::claimAndSwap StrategyArb::claimAndSwap & StrategyMainnet::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 balAfter = asset.balanceOf(address(this));
require((balAfter - balBefore) >= _minOut, "Slippage too high");
@> transmuter.deposit(asset.balanceOf(address(this)), address(this));
}
Transmuterv2::claim:
function claim(uint256 amount, address recipient) external override nonReentrant {
_onlyWhitelisted();
@> _updateAccount(
UpdateAccountParams({
owner: msg.sender,
unexchangedDelta: 0,
exchangedDelta: -SafeCast.toInt256(_normalizeUnderlyingTokensToDebt(amount))
})
);
TokenUtils.safeBurn(syntheticToken, _normalizeUnderlyingTokensToDebt(amount));
ITransmuterBuffer(buffer).withdraw(underlyingToken, amount, recipient);
emit Claim(msg.sender, recipient, amount);
}
Transmuterv2::deposit
function deposit(uint256 amount, address owner) external override nonReentrant {
_onlyWhitelisted();
@> _updateAccount(
UpdateAccountParams({
owner: owner,
unexchangedDelta: SafeCast.toInt256(amount),
exchangedDelta: 0
})
);
TokenUtils.safeTransferFrom(syntheticToken, msg.sender, address(this), amount);
emit Deposit(msg.sender, owner, amount);
}
As you can see in the code, claimAndSwap uses transmuterv2::claim and transmuterv2::deposit consecutively without any input sanitation in claimAndSwap. This results in transmuterv2::mapping(address => Account) private accounts; carrying wrong values which keepers possibly could withdraw.
Impact:
Being able to inflate the keepers balance due to the lack of input sanitation would allow the keeper to withdraw arbitrary amounts out of the protocol.
Likelihood: High
Impact: High
Severity: High
Tools Used:
Manual review.
Recommended Fix:
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IVeloRouter.route[] calldata _path ) external onlyKeepers {
+ uint256 maxClaimable = transmuter.getClaimableBalance(address(this));
+ if(_amountClaim > maxClaimable) {
+ revert TooHighClaim(_amountClaim, maxClaimable);
+ };
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));
}
Implementing this fix will make the claimAndSwap function in the contracts fail directly in the keeper should not have sufficient claimable balance with the transmuter.
