Good Entry - pep7siup's results

Lines of code

https://github.com/code-423n4/2023-08-goodentry/tree/main/contracts/RangeManager.sol#L100

Vulnerability details

Impact

The initRange function in the RangeManager contract has a vulnerability where leftover underlying assets from the tokenizable range are not properly refunded to the contract owner. This can lead to a situation where the owner's funds are stuck in the contract, and subsequent user interactions could unknowingly deposit this fund into the Lending pool in the user's favor, causing the owner to lose funds.

Proof of Concept

// Findings are labeled with '<= FOUND'
// File: contracts/RangeManager.sol
95:  function initRange(address tr, uint amount0, uint amount1) external onlyOwner {
96:    ...
99:    ASSET_1.safeIncreaseAllowance(tr, amount1);
100:    TokenisableRange(tr).init(amount0, amount1); // <= FOUND: tr.init transfer leftover underlying ASSET_0 & ASSET_1 back to RangeManager contract

// File: contracts/TokenisableRange.sol
134:  function init(uint n0, uint n1) external {
135:    ...
158:    // Transfer remaining assets back to user
159:    TOKEN0.token.safeTransfer(msg.sender, TOKEN0.token.balanceOf(address(this))); // <= FOUND
160:    TOKEN1.token.safeTransfer(msg.sender, TOKEN1.token.balanceOf(address(this))); // <= FOUND
161:    _mint(msg.sender, 1e18);
162:    emit Deposit(msg.sender, 1e18);
163:    ...

1. The owner of the RangeManager contract uses the initRange function to initialize a tokenizable range (TokenisableRange contract).
2. The tr contract transfers leftover underlying assets back to the RangeManager in its init function.
3. The initRange function attempts to transfer the minted TR balance to the owner's address.
4. Excess underlying assets are left in the RangeManager
5. Subsequent user interactions with the contract, specifically the ones that can call cleanup function, can deposit the owner's leftover funds to the Lending pool in the user's favor.

Tools Used

Manual review

Recommended Mitigation Steps

Update the initRange function to ensure that leftover underlying assets in the RangeManager contract are properly refunded to the contract owner.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-goodentry/tree/main/contracts/helper/V3Proxy.sol#L156 https://github.com/code-423n4/2023-08-goodentry/tree/main/contracts/helper/V3Proxy.sol#L174 https://github.com/code-423n4/2023-08-goodentry/tree/main/contracts/helper/V3Proxy.sol#L192

Vulnerability details

Impact

The swapETHForExactTokens, swapTokensForExactETH, and swapExactTokensForETH functions in the V3Proxy contract have a vulnerability where the return value of low-level calls is ignored. If a transaction fails during the call to refund unused ETH back to the user, the ETH is not refunded, resulting in a potential loss of funds for the user. This situation could arise due to various reasons, such as out-of-gas errors or contract reverts. In such cases, the user's ETH would not be refunded, leading to a financial loss.

Proof of Concept

1. A user initiates a swap using one of the affected functions (swapETHForExactTokens, swapTokensForExactETH, or swapExactTokensForETH).
2. The swap involves sending a certain amount of ETH to the contract.
3. The contract performs the swap operation and calculates the amount of unused ETH that should be refunded back to the user.
4. The contract attempts to refund the unused ETH using a low-level call.
5. However, for some reason (e.g., out-of-gas error, contract revert), the low-level call fails.
6. As a result, the user's unused ETH is not refunded, leading to a loss of funds for the user.

Tools Used

Manual review

Recommended Mitigation Steps

Check the return value of the low-level call that refunds unused ETH to the user. If the low-level call fails, the contract should revert the transaction and emit an error message indicating that the refund failed.

Assessed type

ETH-Transfer

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/PositionManager/OptionsPositionManager.sol#L236-L239 https://github.com/code-423n4/2023-08-goodentry/tree/main/contracts/PositionManager/OptionsPositionManager.sol#L310

Vulnerability details

Impact

In the OptionsPositionManager contract, there is a vulnerability related to the treatment of excess repayment amount when closing a user's position. The contract does not take into account the final amount repaid from the LP.repay function (Line 310), potentially leading to an excess amount of debt remaining in the contract. This issue could result in a loss of user funds.

function close(...) 
  external
{
  ...
  debt = closeDebt(poolId, user, debtAsset, debt, collateralAsset);

  cleanup(LP, user, token0); // <= Found
  cleanup(LP, user, token1); // <= Found
}

function closeDebt(...) internal returns (uint debt) {
    ...
    if (user != address(this) ) LP.repay( debtAsset, debt, 2, user); // <= Found
}

Proof of Concept

In the scenario where a user tries to repay with an over-estimated amount of debt, the excess TR amount is stuck in the OptionsPositionManager contract. OptionsPositionManager.close function, in charge of asset refunding, does not do its task properly (Line 238-239). It only refund Token0 and Token1, not takes care of potential TR debt. This can lead to a situation where the contract holds an excess amount of debt, which is not accounted for.

Tools Used

Manual review

Recommended Mitigation Steps

Record the actual repaid amount from the LP.repay function and adjust the user's debt accordingly. Additionally, the excess amount that is not refunded should be accounted for and properly handled in the OptionsPositionManager.close function.

Assessed type

Other