Good Entry - digitizeworx's results

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L196 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L201 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L190-L202

Vulnerability details

Impact

Users who have not granted allowance for the contract to spend their tokens might encounter failed deposits, disrupting the expected flow of interactions. Inconsistent behavior and a lack of clarity in token deposits could lead to user confusion and reduced trust in the contract's functionality.

The lines where tokens are deposited into the lending pool without checking the contract's allowance to spend those tokens.

if (asset0_amt > 0) {
  ASSET_0.safeIncreaseAllowance(address(LENDING_POOL), asset0_amt);
  LENDING_POOL.deposit(address(ASSET_0), asset0_amt, msg.sender, 0);
}

if (asset1_amt > 0) {
  ASSET_1.safeIncreaseAllowance(address(LENDING_POOL), asset1_amt);
  LENDING_POOL.deposit(address(ASSET_1), asset1_amt, msg.sender, 0);
}

Proof of Concept

In the cleanup function, you are depositing tokens directly into the lending pool without checking the allowance of the contract to spend those tokens. This might result in the contract being unable to deposit tokens on behalf of the user if the allowance is not set correctly.

This is where tokens are deposited directly into the lending pool without checking the allowance. Line196

LENDING_POOL.deposit(address(ASSET_0), asset0_amt, msg.sender, 0);

and

Line201

LENDING_POOL.deposit(address(ASSET_1), asset1_amt, msg.sender, 0);

In these lines, the contract is depositing asset0_amt and asset1_amt tokens directly into the lending pool on behalf of the user (msg.sender) without explicitly checking if the contract has been allowed by the user to spend those tokens. This can lead to a situation where the contract is unable to deposit tokens on behalf of the user if the allowance is not set correctly, potentially causing unexpected behavior and usability issues.

Imagine two users, Alice and Bob, interacting with the smart contract that contains the cleanup function. Alice has approved the contract to spend 100 tokens of ASSET_0, while Bob has not approved any allowance for the contract.

1. Initial State:

   • Alice's balance of ASSET_0 tokens: 100 tokens
   • Bob's balance of ASSET_0 tokens: 200 tokens
   • Alice's allowance for the contract to spend ASSET_0 tokens: 100 tokens
   • Bob's allowance for the contract to spend ASSET_0 tokens: 0 tokens

2. Interaction 1: Alice Calls removeAssetsFromStep Function

   • Alice calls the removeAssetsFromStep function, triggering the cleanup function.
   • The cleanup function attempts to deposit Alice's ASSET_0 tokens (100 tokens) into the lending pool.

3. Interaction 2: Bob Calls removeAssetsFromStep Function

   • Bob calls the removeAssetsFromStep function, triggering the cleanup function.
   • The cleanup function attempts to deposit Bob's ASSET_0 tokens (200 tokens) into the lending pool.

4. Result:

   • For Alice: The deposit for Alice's tokens succeeds, as she has allowed the contract to spend her tokens.
   • For Bob: The deposit for Bob's tokens fails, as he has not allowed the contract to spend his tokens. The contract does not have the necessary allowance to deposit Bob's tokens into the lending pool.

In this scenario, the issue becomes apparent when Bob interacts with the contract. The cleanup function blindly attempts to deposit tokens into the lending pool without checking the allowance for each user. This results in a discrepancy in behavior between users who have granted allowance and those who have not. As a result, Bob's tokens cannot be deposited, potentially leading to confusion and inconsistent behavior for users.

Tools Used

VsCode

Recommended Mitigation Steps

The contract should first check the allowance of the tokens before attempting to deposit them into the lending pool. If the allowance is insufficient, the contract should guide the user to set the necessary allowance or provide instructions on how to proceed.

Assessed type

Access Control

Analysis Report

GoodEntry is a perpetual options trading platform that is built on top of Uniswap v3. It uses single-tick liquidity to provide users with limited downside exposure. The protocol consists of three main components:

• Tokenized Uniswap v3 positions (TRs)
• ezVaults
• A lending pool

TRs are ERC-20 tokens that represent single-tick liquidity on Uniswap v3. They can be used to trade perps with limited downside. ezVaults are smart contracts that hold TRs and rebalance the underlying liquidity as needed. The lending pool is a forked version of Aave v2 that allows users to borrow and lend TRs.

The core idea behind GoodEntry is that single-tick liquidity in Uniswap behaves as a limit order, whose payout is similar to writing an option. Borrowing such liquidity and removing it from the tick gives a payout similar to buying an option.

The protocol has been audited by several security firms and is now under audit on the platform of Code4rena and it has a high level of code coverage. However, there are still some potential risks that should be considered before using the protocol.

Centralization Risks

The lending pool is a centralized component of the protocol. This means that if the lending pool is hacked, it could lead to the loss of user funds. Additionally, the lending pool is controlled by a single entity, which could lead to censorship or other problems.

Mechanism Review

The mechanism of the protocol is complex and there are some potential risks that should be considered. For example, if the price of the underlying asset moves significantly, it could lead to liquidation of user positions. Additionally, the protocol is not fully collateralized, which means that there is a risk of default.

Systemic Risks

The protocol is designed to be a decentralized platform, but there are some potential systemic risks that should be considered. For example, if a large number of users were to sell their TRs at the same time, it could lead to a crash in the price of the protocol's tokens. Additionally, the protocol is dependent on the underlying liquidity of Uniswap v3, which could pose a risk if the liquidity of Uniswap v3 were to decrease.

GoodEntry is a promising project with a lot of potential.

Recommendations

• The lending pool should be decentralized to reduce the risk of centralization.
• The protocol should be fully collateralized to reduce the risk of default.
• The protocol should be stress-tested to identify and mitigate potential systemic risks.

Conclusion

GoodEntry is a promising project with a lot of potential. However, there are some risks that should be considered before using the protocol. The protocol should be decentralized, fully collateralized, and stress-tested to mitigate the potential risks.

Time spent:

31 hours