search

sherlock-audit / 2022-11-float-capital-judging

2 stars 1 forks source link

ctf_sec - MarketCore#_mint should check if the market is not deprecated #5

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

medium

MarketCore#_mint should check if the market is not deprecated

Summary

MarketCore#_mint should check if the market is not deprecated

Vulnerability Detail

https://github.com/sherlock-audit/2022-11-float-capital/blob/main/contracts/market/template/MarketCore.sol#L246-L257

MarketCore#_mint should check if the market is not deprecated before the minting, as the comment suggested.

  /// @dev We have to check market not deprecated after system state update because that is the function that determines whether the market should be deprecated.

note the function redeem made the check using checkMarketNotDeprecated modifier

  function _redeem(
    uint112 amount,
    address user,
    PoolType poolType,
    uint256 poolTier
  ) internal checkMarketNotDeprecated {

Impact

User's may mint position even after the market is deprecated.

Code Snippet

https://github.com/sherlock-audit/2022-11-float-capital/blob/main/contracts/market/template/MarketCore.sol#L246-L257

Tool used

Manual Review

Recommendation

I am aware that when the market is marked as deprecated, and mint would revert,

    marketDeprecated = true;
    mintingPaused = true;

but by just looking at the code, checkMarketNotDeprecated modifier is still needed in case the admin accidentally unpause the pool.

WooSungD commented 1 year ago

As noted in the recommendation section, mintingPaused is set to true when market is deprecated. Function _mint() is reverted if mintingPaused is true.

checkMarketNotDeprecated modifier is still needed in case the admin accidentally unpause the pool.

Agree that for checkMarketNotDeprecated modifier should be used for _mint() function though for consistency. To be discussed with Float team.

JasoonS commented 1 year ago

There is no way to unpause minting on a market that is deprecated. So I don't see any possibility of issues here. And a deprecated market ALWAYS has minting paused.

Fixing the comments - they were a bit outdated.

image

JasoonS commented 1 year ago

https://github.com/Float-Capital/monorepo/pull/3813 PR with the comment changes from the above image