Boxy Laurel Buffalo
Medium
Notice how the swap
function works in ‘AmirX`. If the input ERC20 tokens are not stablecoin, they are converted to stablecoin by _defiSwap, and then the resulting stablecoin contract is swapped by _stablecoinSwap.
Note, however, that the value of oAmount can be overwritten during the swap
process.
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);
Also note that the _stablecoinSwap internal function does not check the input parameters in any way. The input parameters are checked in the _verifyStablecoinSwap function, which is called at the beginning of swap execution, but is called with the old ss parameters. Since the ss.oAmount parameter can be updated during execution, it is necessary to check ss again, but the protocol does not do this.
Let's see where this goes.
function _verifyStablecoinSwap(
address wallet,
StablecoinSwap memory ss
) internal view nonZero(ss) {
// Ensure the wallet address is not zero.
if (wallet == address(0)) revert ZeroValueInput("WALLET");
// For the origin currency:
if (isXYZ(ss.origin)) {
// Ensure the total supply does not drop below the minimum limit after burning the specified amount.
if (
Stablecoin(ss.origin).totalSupply() - ss.oAmount <
getMinLimit(ss.origin)
) revert InvalidMintBurnBoundry(ss.origin, ss.oAmount);
} else if (ss.liquiditySafe == address(0)) {
// Ensure the liquidity safe is provided for ERC20 origin tokens.
revert ZeroValueInput("LIQUIDITY SAFE");
}
// For the target currency:
if (isXYZ(ss.target)) {
// Ensure the total supply does not exceed the maximum limit after minting the specified amount.
if (
Stablecoin(ss.target).totalSupply() + ss.tAmount >
getMaxLimit(ss.target)
) revert InvalidMintBurnBoundry(ss.target, ss.tAmount);
} else if (ss.liquiditySafe == address(0)) {
// Ensure the liquidity safe is provided for ERC20 target tokens.
revert ZeroValueInput("LIQUIDITY SAFE");
}
}
As we can see, for ss.oAmount it is checked that if it is an XYZ token, then after burning totalSupply - ss.oAmount > minLimit. However, since ss.oAmount can update and become larger than the original value, this invariant can be violated.
_stablecoinswap is called with updated ss, but verifyStableCoinSwap is called for old ss. If you examine the old code of this function from the previous contest, you will notice that the implementation there does not call an internal function, but an external one, which includes the necessary checks. It also intentionally checks that ss.origin is not an XYZ token, which is not done in the current implementation of contracts.
if (!isXYZ(ss.origin) && isXYZ(ss.target)) {
if (fBalance - iBalance != 0) ss.oAmount = fBalance - iBalance;
convertToEXYZ(wallet, safe, ss);
}
As a result, _defiSwap fBalance - iBanalce should be greater than ss.oAmount.
Also totalSupply - ss.oAmount > minLimit, and totalSupply - (fBalance - iBalance) < minLimit.
No external pre conditions
This invariant violation can occur either intentionally on the part of the user or by accidentally sending swap.
Protocol invariant is violated, as a result the minLimit XYZ of the token that is set by MAINTAINER may be violated.
Severity: Medium
No response
Add additional check that ss.oAmount = min(ss.oAmount, fBalance - iBalance) or add verifyStableCoinSwap before _stablecoinSwap
The same error occurs in the defiToStablecoinSwap
function