code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

mint() function: anyone can mint position NFTs without needing any Ajna pool deposits/collateral. Essentially equals "empty" NFTs that cannot be used to stake. #459

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/d80daab705a066828ef1c5d9ba85f315c7c932db/ajna-core/src/PositionManager.sol#L227-L241

Vulnerability details

Impact

The current implementation of the mint function in the smart contract does not require the minter to have any existing liquidity pool (LP) deposits or collateral in the Ajna pool. As a result, an actor can mint position NFTs without backing them with actual LP tokens. These NFTs are essentially "empty" and cannot be used for staking because they do not represent any real value or stake in the liquidity pool. This oversight can lead to an inflation of NFTs that do not represent any real value, distorting the NFT marketplace and devaluing the legitimate NFTs that represent real stakes in the liquidity pool.

Proof of Concept

In the current implementation, the mint function can be invoked without any checks on whether the minter holds any deposits or collateral in the Ajna pool.

Here's an example of how an actor can mint an "empty" NFT:

mint({pool: somePool, poolSubsetHash: someHash, recipient: someAddress});

Even if the actor does not have any deposits or collateral in the somePool, they can still mint a new NFT and assign it to someAddress. This NFT does not represent any real stake in the liquidity pool and is therefore "empty".

The function involved in this scenario is: mint()

Tools Used

VSC.

Recommended Mitigation Steps

To prevent the minting of "empty" NFTs, it is recommended to introduce a check in the mint function to ensure that the minter has existing LP deposits or collateral in the Ajna pool. This could be done through a modifier that checks the LP balance of the minter for the specified pool before the NFT is minted.

Here's a simple example of how such a check could be implemented:

modifier hasDeposit(address minter, address pool) { require(getLpBalance(minter, pool) > 0, "No LP deposit in pool"); _; }

function mint(MintParams calldata params) external override hasDeposit(msg.sender, params.pool) nonReentrant returns (uint256 tokenId_) { // ... rest of the mint function }

In the above code, getLpBalance is a hypothetical function that retrieves the LP balance of an address for a specific pool. You should replace it with the actual function used in your system to check an address's LP balance.

This way, only addresses with an existing LP deposit in the specified pool can mint NFTs, preventing the creation of "empty" NFTs.

Assessed type

Other

Picodes commented 1 year ago

Low severity given the impact described

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-sponsor commented 1 year ago

ith-harvey marked the issue as sponsor disputed

ith-harvey commented 1 year ago

Disputing this as minting multiple NFTs for the same pool is expected functionality.