code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Description

In PrePOMarket.sol, the _finalLongPayout represents the finalized value of a single long token in the market. It is settled post ICO / IPO according to predetermined rules. The issue is that this value may be re-initialized as many times as owner desires:

function setFinalLongPayout(uint256 _finalLongPayout) external override onlyOwner {
  require(_finalLongPayout >= floorLongPayout, "Payout cannot be below floor");
  require(_finalLongPayout <= ceilingLongPayout, "Payout cannot exceed ceiling");
  finalLongPayout = _finalLongPayout;
  emit FinalLongPayoutSet(_finalLongPayout);
}

This introduces a very substantial centralization risk. A malicious owner may set long payout to floorLongPayout and cash out on all their short tokens, which would be worth their maximum potential. Then, they could re-initialize it to ceilingLongPayout and cash out their long tokens. This way, they are directly stealing from long / short token holders which will not be able to claim their underlying holdings.

For reference, this block in redeem() calculates the collateral amount. It clearly shows how the long tokens appreciate in value when finalLongPayout is high, and short tokens appreciate in value when finalLongPayout is low.

uint256 _collateralAmount;
if (finalLongPayout <= MAX_PAYOUT) {
  uint256 _shortPayout = MAX_PAYOUT - finalLongPayout;
  _collateralAmount = (finalLongPayout * _longAmount + _shortPayout * _shortAmount) / MAX_PAYOUT;
} else {
  require(_longAmount == _shortAmount, "Long and Short must be equal");
  _collateralAmount = _longAmount;
}

M - finalLongPayout can be reinitialized many times although by design it is final. Add require. Risk of owner misusing to enjoy high evaluation of Long position and then high evaluation of short position.

Impact

Owner can rug PrePOMarket using re-initialized finalLongPayout.

Tools Used

Manual audit

Recommended Mitigation Steps

finalLongPayout should only be allowed to be set once.

hansfriese commented 1 year ago

duplicate of #231 (my submission)

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #231