PoolTogether - 0xPsuedoPandit's results

Lines of code

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

Vulnerability details

Impact

_yieldFeeTotalSupply can be drained by malicious actor which belongs to the yield fee recipient which was set at the beginning inside the constructor.

Proof of Concept

mintYieldFee have no access control and it is supposed to be called by the actual recipient of the fee which is set inside the constructor and stored in the state variable _yieldFeeRecipient which is the address of the yield fee recipient that receives the fee amount when yield is captured. https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L394-L402 The function is external and anyone can pass his address in the params of mintYieldFee as recipient further this function calls an internal function _mint https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1122-L1123 This function in turn calls _twabController.mint in _twabController.sol which expects mint function to be called by vault and the msg.sender will indeed be the vault in the context of TwabController contract https://github.com/GenerationSoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/TwabController.sol#L457C2-L460C1

finally the fake receiver can get all the shares against _yieldFeeTotalSupply.

Tools Used

manual review

Recommended Mitigation Steps

check if the address of _recipient is same as _yieldFeeRecipient.

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

A malicious actor can get access to crucial functions like withdrawReserve and closeDraw.

Proof of Concept

In PrizePool contract there are two ways to set draw manager either inside the constructor or by setDrawManager which can be called by anyone if it has not been set yet. setDrawManager is wide open to the frontrunners and drawManager can make whopping changes in the protocol like withdrawing reserve and closing draw. It should be set inside the constructor and if it's not constructor then at least this function should have some access control, like it should be called only by the deployer of PrizePool contract, as per the docs there is going to be a single PrizePool contract on every chain and there can be many vaults per chain so the there can a large sum of pool tokens that can be associated with it and chances of this happening is very likely so making this of medium severity.

Tools Used

manual review

Recommended Mitigation Steps

Put Access control on drawManager.

Assessed type

Access Control