Lack of Restriction on ScalarMarket Bounds May Result in Token Loss During Redemption

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x5484c9ddc29a8ae7e0e18a6ce336024cfe2f76495ba9014c93471be0f6ea8875 Severity: medium

Description:

Description

Currently, there is no restriction on the difference between params.upperBound and params.lowerBound in the createScalarMarket function for ScalarMarket. This could cause issues during token redemption, potentially leading to token loss.

Details

The formula used for calculating the total payout when redeeming tokens is as follows:

• payoutStake.mul(payoutNumerator).div(den);
• For UP tokens: payoutStake * (value - min) / (max - min)
• For DOWN tokens: payoutStake * (max - value) / (max - min)

If the difference between max and min is too large and the value is near either max or min, the redemption calculation will produce zero amounts for some users.

• users whose position has a high denominator: they will redeem their tokens
• users whose position has a low denominator: they couldn't redeem their tokens and these tokens will stuck in the contract.

Impact

If the difference between max and min is excessively large, some users may lose tokens. These tokens will become locked and irretrievable in the contract, leading to potential user dissatisfaction and lost funds.

Mitigation

About payoutStake: this is a balance of users, so if assume the token is sDAI it will be balance* 18 decimals.

So in contract creation, the contract could check higher - lower to be less than 1e18, in this way the following state will always be true.

payoutStake.mul(payoutNumerator) > div(den));

To prevent this, impose a restriction on the difference between upperBound and lowerBound during market creation. Ensure that the difference is less than 1e18 to maintain payout calculations within a valid range:

function createScalarMarket(CreateMarketParams calldata params) external returns (address) {
    require(params.upperBound > params.lowerBound, "upperBound must be higher than lowerBound");
+   require(params.upperBound - params.lowerBound < 1e18, "Difference between upperBound and lowerBound is too large");

This will prevent scenarios where the payoutNumerator becomes too large and ensures that payout calculations remain within feasible limits.

[clesaege]

So I think you are referring to this part. We multiply payoutStake and payoutNumerator before dividing, so we won't have high rounding issues.

There can still be negligeable amounts lost due to rounding but this is acceptable. A per policy is excluded: - Negligible losses (such as losses due to rounding).