code423n4 commented 1 year ago

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/PrePOMarket.sol#L65-L74

Vulnerability details

Impact

Function mint() Mints Long and Short tokens in exchange for amount collateral and according to the comment in the IPrePOMarket: "Minting is not allowed after the market has ended." but there is no check or restriction in the code that to make sure minting is not possible after Market is expired. function mint() should have exact modifier which prevented users from depositing after block.timestamp has passed Market's expiryTime. right now after market has expired users still can call mint and deposit tokens.

Proof of Concept

This is mint() code:

  function mint(uint256 _amount) external override nonReentrant returns (uint256) {
    require(finalLongPayout > MAX_PAYOUT, "Market ended");
    require(collateral.balanceOf(msg.sender) >= _amount, "Insufficient collateral");
    if (address(_mintHook) != address(0)) _mintHook.hook(msg.sender, _amount, _amount);
    collateral.transferFrom(msg.sender, address(this), _amount);
    longToken.mint(msg.sender, _amount);
    shortToken.mint(msg.sender, _amount);
    emit Mint(msg.sender, _amount);
    return _amount;
  }

As you can see there is no check that makes mint() not callable after expiryTime. the only check is finalLongPayout > MAX_PAYOUT, but the value of finalLongPayout is change by owner call to setFinalLongPayout() and owner may don't call it even until expiryTime and contract should explicitly restrict access to mint() when block.timestamp >= expirtyTime. it's possible to mint after expirty time which is not the functionality described in docs and comments.

Tools Used

VIM

Recommended Mitigation Steps

add a modifier and restrict mint() from getting called after expirtyTime

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

Picodes commented 1 year ago

Note that from this comment, it seems that the expiryTime is just an info but isn't supposed to be a hard limit or to enforce anything

c4-sponsor commented 1 year ago

ramenforbreakfast marked the issue as sponsor disputed

ramenforbreakfast commented 1 year ago

Yes this was mentioned both in the README and in the comments that expiryTime is currently just there as an on-chain reference value, not enforced. So I would not consider this valid.