Malicious vault owners can steal the PrizePool's Reserves as well as contributions made by EOA to other vaults.

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/main/src/PrizePool.sol#L311-L330
https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/main/src/PrizePool.sol#L498-L502

Vulnerability details

Impact

• PrizePool reserves and contributions made by EOA to other vaults can be stolen.

Proof of Concept

• First of all, let's see how the PrizePool reserves can be manually increased as well as how external entities to the Vault and Liquidators can contribute PrizeTokens to their vaults.

1. Increasing the PrizePool reserves

• The PrizePool reserves can be increased by anyone by calling the PrizePool:increaseReserve() which will transfer the specified amount of PrizeTokens into the PrizePool & will update the _reserve variable to reflect the number of new tokens added as a reserve
  • So, the reserves will be left out as the balance of the PrizePool contract, which means that the prizeToken.balanceOf(address(this)) will return the reserves + any other amount of tokens that have been received on the PrizePool contract.

2. External entities using their EOA contribute PrizeTokens to a specific vault

• The PrizePool:contributePrizeTokens() function allows anybody to contribute PrizeTokens on behalf of any Vault.

  • As an example, the vault's owner could contribute tokens directly to the vault without going through the liquidation process by calling this function.
  • The issue is if the contribution is made in two different transactions, the first one would be to transfer the PrizeToken into the PrizePool contract, and the second one would be to credit those tokens to the desired vault.

• Now that we have more context about how the PrizePool reserves and how PrizeTokens contributions are made, let's analyze the reason for the vulnerability and how it can be exploited.

  • So, for any of the two processes, the end result is an increase in the balance of PrizeTokens that the PrizePool contract holds.

    • That being said, the reserves can be stolen as follows:

    • After the reserves have been increased a malicious vault owner calls the PrizePool:contributePrizeTokens() and sends the _prizeVault as the address of his own vault, and the _amount as the exact amount of reserves that were deposited.

      • The contributePrizeTokens() will assume that the caller has already transferred the _amount of PrizeTokens into the PrizePool contract when calculating the _deltaBalance because is not discounting the _reserves it will leave the exact amount of reserves that the PrizePool contract is holding.
      • From here onwards, the computed value for the _deltaBalance will be used to update the vaultAccumulator & the totalAccumulator variables.

    • As a result, the tokens of the reserve have been stolen and credited to the vault of the malicious owner, and now the PrizePool contract doesn't have enough tokens to back up the reserves that are tracked by the _reserve variable

    • As for stealing other's vault contributions:

      • After the EOA has transferred the tokens into the PrizeToken, an attacker can front-run the second transaction and use the deposited tokens by the vault's owner to credit a vault of himself, in this way, the attacker can front-run all the PrizeToken contributions that are not made in a single tx (or in other words, contributions made by EOA accounts).

  • The attacker can accumulate all the stolen tokens on his vault and increase the _vault's contributions of his vault. Later, the attacker can claim those tokens by calling the Vault::claimPrizes() and passing the parameters in a way that one of his accounts will win the prize and get away with all the stolen PrizeTokens that were used to fund the Prize Pool of that vault.

Tools Used

Manual Audit

Recommended Mitigation Steps

• First of all, make sure to discount the reserves from the _deltaBalance, in this way, the _deltaBalance won't consider the reserves as part of the contributions.

• Also, make sure to add a safe path to cover the edge case when an external entity to the vault (could be the vault's owner) would like to contribute extra Prize Tokens to its vault and wouldn't like to go through the liquidation process

  • This safe path should allow the contributor to transfer the requested amount of PrizeTokens from his account into the PrizePool contract, in this way when contributing tokens without going through the liquidation process, the contribution process will be made in one single tx, which will allow EOA to safely contribute tokens.
  • In this way, EOA accounts won't be forced to first transfer tokens into the Prize Pool, now the Prize Tokens will be transferred as part of the same contributePrizeTokens() function execution

function contributePrizeTokens(address _prizeVault, uint256 _amount) external returns (uint256) {
- uint256 _deltaBalance = prizeToken.balanceOf(address(this)) - _accountedBalance();
+ uint256 _deltaBalance = prizeToken.balanceOf(address(this)) - _accountedBalance() - _reserves;

  if (_deltaBalance < _amount) {
-   revert ContributionGTDeltaBalance(_amount, _deltaBalance);
+   uint256 extraAmount = _amount - deltaBalance;
+   prizeToken.safeTransferFrom(msg.sender, address(this), extraAmount);
  }
  ...
  ...
  ...
}

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #295

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #295

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #200

[c4-judge]

Picodes marked the issue as satisfactory

[stalinMacias]

Hey @Picodes , I just want to confirm that my report is a duplicate of the issue #147

[Picodes]

Yes! The source of truth for this is the current label on the issue.