Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Divide Before Multiply #25

Open
code423n4 opened this issue Aug 22, 2021 · 1 comment
Open

Divide Before Multiply #25

code423n4 opened this issue Aug 22, 2021 · 1 comment

Comments

@code423n4
Copy link
Contributor

Handle

leastwood

Vulnerability details

Impact

The function RCMarket._payoutWinnings() function is used to distribute winnings to each market participant upon calling RCMarket.withdraw(). The function uses _remainingPot as a variable to determine how much winnings to transfer to the market participant. The function performs a division on the _remainingPot variable before multiplying the result by _winnersTimeHeld to generate the _numerator variable. This may potentially lead to small rounding in the amount paid to the market participant.

Proof of Concept

https://github.com/code-423n4/2021-08-realitycards/blob/main/contracts/RCMarket.sol#L572-L586

Tools Used

slither

Recommended Mitigation Steps

Perform the division using the PER_MILLIE variable after the multiplication in uint256 _numerator = _remainingPot * _winnersTimeHeld.

@code423n4 code423n4 added 0 (Non-critical) bug Something isn't working labels Aug 22, 2021
code423n4 added a commit that referenced this issue Aug 22, 2021
@Splidge
Copy link
Collaborator

Splidge commented Aug 23, 2021

We acknowledge there is a rounding issue. However it appears that in the worst case this would cause _remainingPot to be just shy of 1 Wei less than it should be, this is then further divided up amongst the users that held the winning card.
Given how close we are to launch and the minimal impact this would actually have, we have decided not to make any changes to such a sensitive function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants