Anyone can call the burn function in SmartVaultV3.sol

Severity

High Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L169

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L27

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L206C14-L206C39

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L217

Summary

The burn function in SmartVaultV3.sol has only ifMinted modifier , and it's external which means anyone can call it as the SmartVaults share a Euros contract.

Vulnerability Details

A malicious user that already has minted EURs tokens can call the burn function of another SmartVault contract. He can cause the minted variable to be 0. This on the other hand will cause the requiredCollateralValue to equal zero in function calculateMinimumAmountOut(bytes32 _inTokenSymbol, bytes32 _outTokenSymbol, uint256 _amount). This on the other hand will cause the function to return 0.

    function calculateMinimumAmountOut(bytes32 _inTokenSymbol, bytes32 _outTokenSymbol, uint256 _amount)
        private
        view
        returns (uint256)
    {
        ISmartVaultManagerV3 _manager = ISmartVaultManagerV3(manager);
        uint256 requiredCollateralValue = minted * _manager.collateralRate() / _manager.HUNDRED_PC();
        uint256 collateralValueMinusSwapValue =
            euroCollateral() - calculator.tokenToEur(getToken(_inTokenSymbol), _amount);
        return collateralValueMinusSwapValue >= requiredCollateralValue
            ? 0
            : calculator.eurToToken(getToken(_outTokenSymbol), requiredCollateralValue - collateralValueMinusSwapValue);
    }

And now we get to swap function, where minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount); Meaning minimumAmountOut = 0 which according to the UniswapDocs:

amountOutMinimum: we are setting to zero, but this is a significant risk in production. For a real deployment, this value should be calculated using our SDK or an onchain price oracle - this helps protect against getting an unusually bad price for a trade due to a front running sandwich or another type of price manipulation

Which will cause to loss of funds while swapping.

Impact

High as it can lead to loss of funds.

Tools Used

Manual review

Recommendations

Inctroduce onlyOwner modifier.