code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L962-L970

Vulnerability details

Impact

If an overflow happens, the computed value of the vault portion will be a totally different value than what should really be, which would end up causing the rewards to disburse to be lower for the draw that was used to compute the vault portion.

Note:

Even though the bot reported issues about downcasting variables, it didn't mention this specific unsafe casting, which if an overflow occurs could cause a huge impact on the calculation of the vault portions.

Proof of Concept

When computing the vaultPortion to the PrizePool over a specific duration in draw, the values of the vaultContributed & totalContributed variables are computed on the DrawAccumulatorLib library, and they are computed and returned as uint256 values.
The issue is that in the PrizePool::_getVaultPortion() the vaultContributed & totalContributed variables are unsafely casted from uint256 to int256, which could lead to a silent overflow if any of the two original values don't fit on an int256
- As a result of a silent overflow, the computed value of the vault portion will be a totally different value than what should really be, which would end up causing the rewards to disburse to be lower for the draw that was used to compute the vault portion.

Tools Used

Manual Audit

Recommended Mitigation Steps

Make sure to implement a safe cast that checks if overflows occur to prevent computing a totally different value than what it should really be.
- Use OZ safeCast library for this type of operation.