hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

DOS in the process of redemption(related to conditional token contract) #131

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0xd70b066ed7609245ec97d9a6c7f935b4fc5824ddbf9b236fb09857215e46b311 Severity: medium

Description:

Description

This issue is related to Conditional tokens so I want to use this competition to submit my issue on their contract.

currently the way that conditional token handle the redemption is as follows:

            uint payoutStake = balanceOf(msg.sender, positionId);
            if (payoutStake > 0) {
                totalPayout = totalPayout.add(payoutStake.mul(payoutNumerator).div(den));
                _burn(msg.sender, positionId, payoutStake);
            }

getting the balance of the sender and calculating totalPayout.

Both payoutStake and payoutNumerator are uint256 numbers, so there is a chance that payoutNumerator will be bigger than uint128.max.

The issue here is getting the balance is not a good way to handle this process, if payoutNumerator is a big number, the attacker could send some tokens to the sender, and DOS the process.

ex: in the case of Seer, the attacker could send tokens to Routers and make them unusable for redemption of that specific position.

Impact

Make DOS on integrated contracts in a way that couldn't operate redemption.

Mitigation

get an amount as an input instead of calculating the balance of the user.

clesaege commented 1 month ago

Similar to https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/issues/100

0xmahdirostami commented 1 month ago

@clesaege

Thanks, but this issue is about conditioanl token contract. This contract may be utilized by multiple protocols.

Some protocols may offer high payouts, so calculating the balance of msg.sender is not a good idea(could cause overflow). It's better to receive amounts as an input.

it creates an attack path for an attacker in integrated protocols.

clesaege commented 1 month ago

As per competition rules, are excluded:

0xmahdirostami commented 1 month ago

@clesaege

we'll help you report them to their respective projects

clesaege commented 1 month ago

For conditional tokens, the rationale is similar to that of #100. This is the kind of thing which must be solved at frontend level as there isn't a fixed value of a maximum amount of tokens.