Summary
The token manager contract is used to allow and disallow valid tokens within the protocol. However due to lack of due diligence, disallowing a token can prevent vault owners from withdrawing collateral even under valid conditions.
Vulnerability Details
The token manager contract is used to allow and disallow valid tokens within the protocol.
function addAcceptedToken(address _token, address _chainlinkFeed) external onlyOwner {
ERC20 token = ERC20(_token);
bytes32 symbol = bytes32(bytes(token.symbol()));
for (uint256 i = 0; i < acceptedTokens.length; i++) if (acceptedTokens[i].symbol == symbol) revert TokenExists(symbol, _token);
Chainlink.AggregatorV3Interface dataFeed = Chainlink.AggregatorV3Interface(_chainlinkFeed);
acceptedTokens.push(Token(symbol, _token, token.decimals(), _chainlinkFeed, dataFeed.decimals()));
emit TokenAdded(symbol, _token);
}
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);
}
}
}
However, due to lack of due diligence, removing tokens where vault owners have deposited collateral can render a vault non-operable by preventing
Vault owners from withdrawing collateral.
POC- run this test:
describe('removing collateral', async () => {
it('wrongful remove token', async () => {
const Tether = await (await ethers.getContractFactory('ERC20Mock')).deploy('Tether', 'USDT', 6);
const clUsdUsdPrice = 100000000;
const ClUsdUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('USD / USD');
await ClUsdUsd.setPrice(clUsdUsdPrice);
await TokenManager.addAcceptedToken(Tether.address, ClUsdUsd.address);
const tetherValue = 1000000000;
const ethValue = ethers.utils.parseEther('1');
await Tether.mint(Vault.address, tetherValue);
await TokenManager.removeAcceptedToken(ethers.utils.formatBytes32String('USDT'));
let remove = Vault.connect(user).removeCollateral(ethers.utils.formatBytes32String('USDT'),tetherValue / 2, user.address);
await expect(remove).to.be.revertedWith('err-invalid-token');
});
});
Lead to liquidations if for example the only token used to collateralize a vault is removed. POC- run this test:
it('wrongful remove token leads to liquidation', async () => {
const ethCollateral = ethers.utils.parseEther('0.5');
const wbtcCollateral = BigNumber.from(1_000_000);
const usdcCollateral = BigNumber.from(500_000_000);
await holder5.sendTransaction({to: SmartVaultManager.address, value: ethCollateral});
await WBTC.mint(SmartVaultManager.address, wbtcCollateral);
await USDC.mint(SmartVaultManager.address, usdcCollateral);
const tstStake1 = ethers.utils.parseEther('1000');
const eurosStake1 = ethers.utils.parseEther('2000');
await TST.mint(holder1.address, tstStake1);
await EUROs.mint(holder1.address, eurosStake1);
await TST.connect(holder1).approve(LiquidationPool.address, tstStake1);
await EUROs.connect(holder1).approve(LiquidationPool.address, eurosStake1);
await LiquidationPool.connect(holder1).increasePosition(tstStake1, eurosStake1)
const tstStake2 = ethers.utils.parseEther('4000');
const eurosStake2 = ethers.utils.parseEther('3000');
await TST.mint(holder2.address, tstStake2);
await EUROs.mint(holder2.address, eurosStake2);
await TST.connect(holder2).approve(LiquidationPool.address, tstStake2);
await EUROs.connect(holder2).approve(LiquidationPool.address, eurosStake2);
await LiquidationPool.connect(holder2).increasePosition(tstStake2, eurosStake2);
await fastForward(DAY);
await TokenManager.removeAcceptedToken(ethers.utils.formatBytes32String('WBTC'));
await expect(LiquidationPoolManager.runLiquidation(TOKEN_ID)).not.to.be.reverted;
});
Impact
Vault owners are unable to withdrawing collateral even under valid conditions.
Vault owners are exposed to liquidations and loss of collateral if for example the only token used to collateralize a vault is removed.
Tools Used
Manual review
Recommendations
Check that no vault exists that has owner collateral before removing.
