The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: low
Valid

Removing a token from the token manager may result in the borrower's assets being liquidated prematurely

Summary

The vault obtains the token list from the tokenManager. When a token is removed from the tokenManager, its value is not immediately taken into account. If a borrower deposits this token as collateral, it may lead to the premature liquidation of the borrower's assets.

Vulnerability Details

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L67#L73
vault obtains the token list from the tokenManager

function euroCollateral() private view returns (uint256 euros) {
ITokenManager.Token[] memory acceptedTokens = getTokenManager().getAcceptedTokens();
for (uint256 i = 0; i < acceptedTokens.length; i++) {
ITokenManager.Token memory token = acceptedTokens[i];
euros += calculator.tokenToEur(token, getAssetBalance(token.symbol, token.addr));//73097361300655045
}
}

tokenManager implement removeAcceptedToken function

function removeAcceptedToken(bytes32 _symbol) external onlyOwner {
require(_symbol != NATIVE, "err-native-required");
for (uint256 i = 0; i < acceptedTokens.length; i++) {
if (acceptedTokens[i].symbol == _symbol) {
acceptedTokens[i] = acceptedTokens[acceptedTokens.length - 1];
acceptedTokens.pop();
emit TokenRemoved(_symbol);
}
}
}

Assuming

  • alice deposit 100e18 arb and 0.1 ether as collateral

  • alice mint 250e18 euros

  • owner removed arb from token manager

  • alice's asset is undercollateralised

Here is my test written using foundry

function testRemoveAcceptedTokenLeadToBeLiquidated() public {
//forge test --match-test testRemoveAcceptedTokenLeadToBeLiquidated --fork-url wss://arbitrum-one.publicnode.com -vvv
address arb = address(0x912CE59144191C1204E64559FE8253a0e49E6548);
tokenManager.addAcceptedToken(arb, arbUsd);//add accepted token.
address deploy = address(0x53509eF0e49c8a386b81093711aF1eF29357cc25);
address index = address(0x56c7506410e5e242261c5E0db6941956c686E5A1);
//smart vault manager init.
svIndex = new SmartVaultIndex();
//set manager.
svIndex.setVaultManager(address(svManager));
//grantRole to svManager.
EUROs.grantRole(bytes32(0x00),address(svManager));
svDeployer = new svDeployerV3.SmartVaultDeployerV3(bytes32('ETH'),address(eurUsd));
svManager.initialize(120000,500,address(EUROs),address(protocol),address(0),address(tokenManager),address(svDeployer),address(svIndex),address(0));
//assuming alice is a borrower who create a vault.
vm.startPrank(alice);
(address vault,) = svManager.mint();
//alice get 100e18 arb token.
deal(arb,alice,100e18);//$170.
vm.deal(alice,0.1 ether);//$200.
//alice add collateral.
IERC20(arb).transfer(vault, 100e18);
address(vault).call{value:0.1 ether}("");
//alice borrow euros
ISmartVault(vault).mint(alice,250e18);//$250.
bool undercollateralised = ISmartVault(vault).undercollateralised();
assertEq(undercollateralised,false);
//owner remove accepted token.
vm.stopPrank();
vm.startPrank(address(this));
tokenManager.removeAcceptedToken(bytes32('ARB'));
//check undercollateralised
undercollateralised = ISmartVault(vault).undercollateralised();
assertEq(undercollateralised,true);
}

As we can see after arb is removed from list alice's asset being undercollateralised.
full test file:
https://gist.github.com/coffiasd/3355b9f12307fd588ff5201fccd05192

Impact

borrower's asset being liquidated prematurely

Tools Used

Foundry

Recommendations

recommend to record token list in vault contract

Updates

Lead Judging Commences

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

remove-token

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

removetoken-low

Support

FAQs

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