code-423n4 / 2022-12-forgotten-runiverse-findings

0 stars 0 forks source link

setPlotsAvailablePerSize does not work correctly #7

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-forgotten-runiverse/blob/00a247e70de35d7a96d0ce03d66c0206b62e2f65/contracts/RuniverseLandMinter.sol#L513

Vulnerability details

Impact

The function setPlotsAvailablePerSize can be used for two things:

  1. Decreasing the number of plots that is available for a certain size
  2. Increase the number of plots that is available for a certain size

However, in both cases it can introduce errors that can brick parts of the contract. In case 1 where the number of plots is decreased, it is possible that the new number is smaller than plotsMinted for a specific ID. This will cause an underflow in getAvailableLands, which subtracts these values:

        plotsAvailableBySize[0] = plotsAvailablePerSize[0] - plotsMinted[0];
        plotsAvailableBySize[1] = plotsAvailablePerSize[1] - plotsMinted[1];
        plotsAvailableBySize[2] = plotsAvailablePerSize[2] - plotsMinted[2];
        plotsAvailableBySize[3] = plotsAvailablePerSize[3] - plotsMinted[3];
        plotsAvailableBySize[4] = plotsAvailablePerSize[4] - plotsMinted[4];

On the other hand, when the number of available plots per size is increased, the minting can fail. This can happen because the MAX_SUPPLY in RuniverseLand is set to 70000, which is also the sum of all plotsAvailablePerSize entries. When the sum of these entries is increased beyond 70000, some tokens cannot be minted, because the MAX_SUPPLY is already reached.

Proof Of Concept

plotsAvailablePerSize[0] is changed by the owner to 54500, all other entries are kept the same. Because more users prefer cheap plots, they buy all 54500 plots of size 8x8 and 15500 plots of size 16x16. However, this means that 70000 tokens are minted and no one can buy the 32x32 or 64x64, leading to a substantial financial loss for the protocol (because larger investors might have bought them later, but can no longer do so).

Recommended Mitigation Steps

Enforce that

  1. All entries are larger than plotsMinted
  2. The new entries sum up to 70000 (or to a number that is <= 70000 if decreasing the max supply should be allowed)
GalloDaSballo commented 1 year ago

Lack of check to guarantee invariant

c4-sponsor commented 1 year ago

msclecram marked the issue as sponsor acknowledged

GalloDaSballo commented 1 year ago

Similarly to #10 and #11, the Warden has shown a way for an invariant to be broken based on configuration.

Because certain aspects of the codebase are using the invariants which can be bypassed as shown above, I agree with Medium Severity

c4-judge commented 1 year ago

GalloDaSballo marked the issue as selected for report

msclecram commented 1 year ago

We updated the code with the next changes: -We removed setPlotsAvailablePerSize

https://github.com/bisonic-official/plot-contract/commit/ea8abd7faffde4218232e22ba5d8402e37d96878