Young Shamrock Cottonmouth
Medium
The AmirX::swap()
function performs different sets of swaps depending on the provided parameters.
Before performing the actual swaps, the function verifies the provided parameters in the _verifyStablecoinSwap()
and _verifyDefiSwap()
functions.
The _verifyDefiSwap()
is only executed if defi.walletData.length != 0
meaning the verification is skipped when defi.walletData.length == 0
function swap(
address wallet,
bool directional,
StablecoinSwap memory ss,
DefiSwap memory defi
) external payable onlyRole(SWAPPER_ROLE) whenNotPaused {
// checks if it will fail
if (ss.destination != address(0)) _verifyStablecoinSwap(wallet, ss);
@> if (defi.walletData.length != 0) _verifyDefiSwap(wallet, defi);
if (directional) {
// if only defi swap
@> if (ss.destination == address(0)) _defiSwap(wallet, defi);
In other parts of the contract, the logic makes sure to execute _defiSwap()
only when defi.walletData
contains data.
if (directional) {
// if only defi swap
if (ss.destination == address(0)) _defiSwap(wallet, defi);
else {
// if defi then stablecoin swap
//check balance to adjust second swap
uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
@> if (defi.walletData.length != 0) _defiSwap(wallet, defi);
uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
//change balance to reflect change
if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
_stablecoinSwap(wallet, ss);
}
} else {
// if stablecoin swap
_stablecoinSwap(wallet, ss);
// if only stablecoin swap
@> if (defi.walletData.length != 0) _defiSwap(wallet, defi);
}
However, this oversight still allows for _defiSwap()
to be executed given empty defi.walletData
in certain scenarios.
Since the data passed to wallet.call{value: 0}(defi.walletData)
is empty, the fallback function of the multisig wallet will be triggered. In case it succeeds without modifying any state, only the _feeDispersal()
function will be triggered.
This function performs the fee collection.
function _defiSwap(address wallet, DefiSwap memory defi) internal {
//user's interaction
(bool walletResult, ) = wallet.call{value: 0}(defi.walletData);
require(walletResult, "AmirX: wallet transaction failed");
@> _feeDispersal(defi);
}
Fees should be collected in return of a service provided by the protocol. This flow is problematic because it may execute the fee collection mecanism while no swap has been performed.
None
None
The swap()
function is called with right parameters but with defi.walletData == ""
(empty), directional == true
and ss.destination == address(0)
.
The if (defi.walletData.length != 0) _verifyDefiSwap(wallet, defi);
verification will be skipped since defi.walletData.length == 0
.
Next, if (ss.destination == address(0)) _defiSwap(wallet, defi);
will be executed which won't perform any swap but still trigger _feeDispersal()
Fees are unfairly collected by the protocol while no swap was performed.
No response
In swap()
make sure the _defiSwap()
function is executed only if walletData
is not empty.
if (directional) {
// if only defi swap
if (ss.destination == address(0)) {
@> if(defi.walletData.length != 0) _defiSwap(wallet, defi);
@> else revert();
}
else {
// if defi then stablecoin swap
//check balance to adjust second swap
uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
if (defi.walletData.length != 0) _defiSwap(wallet, defi);
uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
//change balance to reflect change
if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
_stablecoinSwap(wallet, ss);
}
}