Position is not minted in the OwnershipNFTs upon mint in the Rust implementation

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/sol/SeawaterAMM.sol#L346

Vulnerability details

Impact

Currently the user can mint a new position using mintPositionBC5B086D() function in the proxy which will be delegated later in the implementation. The problem is that in the Rust code a new position is created but the actual ERC721 tokenId is not minted. Therefore, the owner will not be authorized to call any function in the OwnershipNFTs as he's not the owner of the tokenId.

Proof of Concept

The current implementation of the mint functionality:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/sol/SeawaterAMM.sol#L346-348

    function mintPositionBC5B086D(address /* token */, int32 /* lower */, int32 /* upper */) external returns (uint256 /* id */) {
        directDelegate(_getExecutorPosition());
    }

First, we call mintPositionBC5B086D() that will be delegated to the implementation contract. In the implementation, it basically grants a new position to the owner:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L506-508

let owner = msg::sender();
self.grant_position(owner, id);

But the actual NFT is then not minted in the OwnershipNFTs smart contract meaning the owner will not be set as ownerOf(tokenId) and the position is not actually created.

Tools Used

Manual review.

Recommended Mitigation Steps

Mint a new ERC721 position in the SeawaterAMM mintPositionBC5B086D() function if the call to the implementation was successful.

Assessed type

Other