code-423n4 / 2021-10-pooltogether-findings

0 stars 0 forks source link

`PrizeSplitStrategy.distribute()` Potentially Distributes Prize and Emits Wrong Event #64

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

leastwood

Vulnerability details

Impact

The PrizeSplitStrategy.distribute() function is called by any user to first capture the award balance and then distribute the pool prize splits. Due to rounding and potentially erroneous split calculations, it may be possible that the amount distributed is less than the prize amount provided as an input to the function. This amount is left within the contract and could lead to wasted awards if not properly distributed. Consequently, the PrizeSplitStrategy.distribute() contract emits the event Distributed with a potentially incorrect prize distribution.

Proof of Concept

https://github.com/pooltogether/v4-core/blob/master/contracts/prize-strategy/PrizeSplitStrategy.sol#L51-L61 https://github.com/pooltogether/v4-core/blob/master/contracts/prize-strategy/PrizeSplit.sol#L148-L164

Tools Used

Manual code review

Recommended Mitigation Steps

Consider returning _prizeTemp instead of _prize in PrizeSplit._distributePrizeSplits() and have the PrizeSplitStrategy.distribute() utilise this result when returning and emitting its event Distributed.

asselstine commented 3 years ago

https://github.com/pooltogether/v4-core/pull/245

PierrickGT commented 3 years ago

Duplicate of https://github.com/code-423n4/2021-10-pooltogether-findings/issues/31

GalloDaSballo commented 3 years ago

The finding is valid as the event was emitting the totalAmount of tokens rather than the actual amount distributed As per the code4rena documentation am downgrading severity to 1 (low-risk): 1 — Low: Low: Assets are not at risk. State handling, function incorrect as to spec, issues with comments.

GalloDaSballo commented 3 years ago

Duplicate of #35