The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Underreported `minted` in ``SmartVaultV3::burn` Leads to User Losses and Vault DOS

eta

Summary

Incorrect minted value calculation: The burn function in the SmartVaultV3 contract doesn't account for fees, leading to underestimation of the minted value. In the SmartVaultV3 contract, the burn function subtracts only the _amount from minted, but it doesn't add the fee as the mint function does. This leads to an inconsistency where minted doesn't accurately reflect the total amount of tokens burned and the fee, because the actual minted = minted - _amount + fee . This impacts over 56% of relevant functions in the SmartVaultV3 contract and some functions in SmartVaultManagerV5 contract, such as undercollateralised, fullyCollateralised, canRemoveCollateral, calculateMinimumAmountOut, liquidate, mint, removeAsset, swap and tokenURI.

SmartVaultV3::burn

function burn(uint256 _amount) external ifMinted(_amount) {
uint256 fee = _amount * ISmartVaultManagerV3(manager).burnFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
- minted = minted - _amount;
+ minted = minted - _amount + fee;
EUROs.burn(msg.sender, _amount);
IERC20(address(EUROs)).safeTransferFrom(msg.sender, ISmartVaultManagerV3(manager).protocol(), fee);
emit EUROsBurned(_amount, fee);
}

It is possible to handle fees by following the same approach as the mint function.

SmartVaultV3::mint

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);
}

Vulnerability Details

If minted is calculated like minted = minted - _amount, it will underestimate minted, which will affect the results of functions such as undercollateralised, fullyCollateralised, canRemoveCollateral, and calculateMinimumAmountOut, and in turn directly affect the results of more than 56% of relevant functions in SmartVault contract and tokenURI function in the SmartVaultManagerV5. This will lead to the vault being unable to provide normal services and user funds being lost, as follows in detail:

1.The calculateMinimumAmountOut function will return an incorrect amount. This means that users may swap the less amount of collateral than they could.

SmartVaultV3::calculateMinimumAmountOut

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

SmartVaultV3::swap

function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
}

2.The undercollateralised function will return false even if the vault is undercollateralised, because the minted is underestimated and the actual minted > maxMintable. This means that the vault may not be liquidated by the functions of liquidate, liquidateNative, liquidateERC20, even though it has insufficient collateral to cover its debt.
SmartVaultV3::undercollateralised

function undercollateralised() public view returns (bool) {
return minted > maxMintable();
}

SmartVaultV3::liquidate

function liquidate() external onlyVaultManager {
require(undercollateralised(), "err-not-liquidatable");
​
}

In addition, the fullyCollateralised function will return true even if the vault is not fully collateralised. This means that users may mint new collateral and could not burn more amount even though the vault has insufficient collateral to support it.

[SmartVaultV3::fullyCollateralised]https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L156C1-L158C6

function fullyCollateralised(uint256 _amount) private view returns (bool) {
return minted + _amount <= maxMintable();
}

[SmartVaultV3::mint]https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L162C9-L162C65

function mint(address _to, uint256 _amount) external onlyOwner ifNotLiquidated {
require(fullyCollateralised(_amount + fee), UNDER_COLL);
}

3.The canRemoveCollateral function will return true even if the vault could not allow users to remove collateral. This means that users may be able to remove collateral even though they do not have the right to do so.

SmartVaultV3::canRemoveCollateral

function canRemoveCollateral(ITokenManager.Token memory _token, uint256 _amount) private view returns (bool) {
return currentMintable >= eurValueToRemove &&
minted <= currentMintable - eurValueToRemove;
}

SmartVaultV3::removeCollateralNative, removeCollateral, removeAsset

function removeCollateralNative(uint256 _amount, address payable _to) external onlyOwner {
require(canRemoveCollateral(getTokenManager().getToken(NATIVE), _amount), UNDER_COLL);
}
​
function removeCollateral(bytes32 _symbol, uint256 _amount, address _to) external onlyOwner {
require(canRemoveCollateral(token, _amount), UNDER_COLL);
}
​
function removeAsset(address _tokenAddr, uint256 _amount, address _to) external onlyOwner {
if (token.addr == _tokenAddr) require(canRemoveCollateral(token, _amount), UNDER_COLL);
}

4.If minted is underestimated, the data in the status struct will be incorrect. This will prevent the SmartVaultManagerV5 contract from generating the correct NFTMetadata through tokenURI function.

SmartVaultV3::status

function status() external view returns (Status memory) {
return Status(address(this), minted, maxMintable(), euroCollateral(),
getAssets(), liquidated, version, vaultType);
}

SmartVaultManagerV5::tokenURI

function tokenURI(uint256 _tokenId) public view virtual override returns (string memory) {
ISmartVault.Status memory vaultStatus = ISmartVault(smartVaultIndex.getVaultAddress(_tokenId)).status();
return INFTMetadataGenerator(nftMetadataGenerator).generateNFTMetadata(_tokenId, vaultStatus);
}

Impact

Inaccurate liquidation: Undercollateralized vaults might not be liquidated, risking debt coverage.
Incorrect minting/burning: Users could mint new collateral when insufficient, or be unable to burn more when needed.
Unauthorized collateral removal: Users could remove collateral without the right to do so.
Incorrect swap amounts: Users might swap less collateral than intended.
Incorrect NFTMetadata: The SmartVaultManagerV5 contract might generate inaccurate NFTMetadata.

Tools Used

Manual

Recommendations

Modify burn function: Add the fee to the minted calculation: minted = minted - _amount + fee.

function burn(uint256 _amount) external ifMinted(_amount) {
- minted = minted - _amount;
+ minted = minted - _amount + fee;
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

fee-loss

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

fee-loss

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.