code-423n4 / 2023-08-pooltogether-mitigation-findings

0 stars 0 forks source link

H-08 MitigationConfirmed #41

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/main/src/PrizePool.sol#L340-L347 https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/main/src/PrizePool.sol#L314-L318

Vulnerability details

Original Issue

H-08 - Increasing reserves breaks PrizePool accounting

Details

The previous implementation to increase reserves in the PrizePool contract didn't take into account the injected reserves, which caused the accounted balance in the prize pool to not be properly updated. The original finding explains in detail how not updating correctly the accounted balance when increasing reserves allowed a vault to effectively steal the prize token contribution by double counting the initial injection into the reserves.

Mitigation

The intention of the implemented mitigation is to keep track of all the injected reserves that happens when executing the contributeReserve(), to achieve this, a new variable is added, the directlyContributedReserve, which is updated each time a contribution is made.

function contributeReserve(uint96 _amount) external {
  _reserve += _amount;
  //@audit-info => Keep track of all the injected contributions.
  directlyContributedReserve += _amount;
  prizeToken.safeTransferFrom(msg.sender, address(this), _amount);
  emit ContributedReserve(msg.sender, _amount);
}

Conclusion

Assessed type

Context

stalinMacias commented 1 year ago

Just want to clarify that this report confirms that the mitigation is correct. Please ignore the labels of High Risk and Bug

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory