code-423n4 / 2023-07-pooltogether-findings

12 stars 7 forks source link

Yield fee is minted out of thin air #418

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L573-L575 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L584

Vulnerability details

Impact

Any call to liquidation gives some share of the liquidated tokens to the yield fee recipient. These shares are not taken from the liquidator, but are newly minted.That means these tokens are essentially minted out of thin air.

Proof of Concept

The user liquidates the tokens and mint some fees to the Vault. Now the vault is undercollateralized and no more deposits or mints are allowed. Overtime the yield generated will slow collateralize the protocol again, but since some operations are forbidden this is an temporary dos attack and the protocol will receive an reputational damage

Tools Used

Manual review

Recommended Mitigation Steps

Deduct the yieldFee from the liquidator fee to keep the vault fully collateralized.

+   fee = _amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut;
    if (_yieldFeePercentage != 0) {
      _increaseYieldFeeBalance(
+       fee
-       (_amountOut * FEE_PRECISION) / (FEE_PRECISION - _yieldFeePercentage) - _amountOut
      );
    }

    uint256 _vaultAssets = IERC20(asset()).balanceOf(address(this));

    if (_vaultAssets != 0 && _amountOut >= _vaultAssets) {
      _yieldVault.deposit(_vaultAssets, address(this));
    }

+    _mint(_account, _amountOut - fee);
-    _mint(_account, _amountOut);

    return true;
  }

Assessed type

Other

code423n4 commented 1 year ago

Withdrawn by markus_ether

code423n4 commented 1 year ago

Withdrawn by markus_ether