MintHook doesn't allow users with NFT score requirement to mint

[code423n4]

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/DepositHook.sol#L45 https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/MintHook.sol#L16

Vulnerability details

Impact

The depositHook contract checks that users are allowed to deposit either if they are on the allow list or if they have the required NFT score:

if (!_accountList.isIncluded(_sender)) require(_satisfiesScoreRequirement(_sender), "depositor not allowed");

However, once the user has deposited base tokens for collateral tokens, they aren't able to use the protocol, because the mintHook only checks the allow list:

function hook(address sender, uint256 amountBeforeFee, uint256 amountAfterFee) external virtual override onlyAllowedMsgSenders { 
    require(_accountList.isIncluded(sender), "minter not allowed"); 
}

This defeats the purpose of the NFT Score Requirement.

Proof of Concept

If a user has the required NFTs they are allowed to call the deposit() function, but not the mint() function, so they aren't actually able to use the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a similar check to the mintHook as exists in the depositHook.

[c4-sponsor]

ramenforbreakfast marked the issue as sponsor disputed

[ramenforbreakfast]

So we tried to communicate this on discord better, but we admittedly didn't do a great job of communicating, outside of documentation, that mint on a PrePOMarket is meant to only be called by the team. The actual trading of positions will occur on Univ3 AMM pools, with the team providing the supply.

[Picodes]

@ramenforbreakfast I have to admit that I am a bit confused because, from the team messages (for example https://discord.com/channels/810916927919620096/1050501091516743690/1051139167284899972), it seems that this NFT score feature was supposed to be used. So based on this message, this finding would be valid?

On the other side we have this comment which says that only the team should mint.

[c4-judge]

Picodes marked the issue as primary issue

[c4-judge]

Picodes marked the issue as selected for report

[ramenforbreakfast]

@ramenforbreakfast I have to admit that I am a bit confused because, from the team messages (for example https://discord.com/channels/810916927919620096/1050501091516743690/1051139167284899972), it seems that this NFT score feature was supposed to be used. So based on this message, this finding would be valid?

On the other side we have this comment which says that only the team should mint.

So NFT scoring is used, but its only within Collateral deposit, which allows users to mint collateral to use for trading. It is not included in any of the code related to PrePOMarket mint. Users mint Collateral to trade on our Uniswap pools, where they can purchase positions minted by the team.

This person is assuming that NFT scoring should also be applied to PrePOMarket mint, which is not the case, NFT scoring will not be used to determine who can mint positions. Only the team will mint positions and users purchase positions from the pools (LP'd by the team).

[Picodes]

Thanks a lot for the clarification @ramenforbreakfast. Closing as invalid.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid