search

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

12 stars 7 forks source link

Some tokens may revert when zero value transfers are made (Addendum) #251

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L832 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1027 https://github.com/GenerationSoftware/pt-v5-vault/blob/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L1155

Vulnerability details

Note: This risk has been included in the Automated Findings. At that time, the findings given by MEDIUM-7 were not comprehensive. I'm of the opinion that the subsequent findings should also be categorized under MEDIUM-7 and they share a similar level of risk.

Impact

In spite of the fact that EIP-20 states that zero-valued transfers must be accepted, some tokens, such as LEND will revert if this is attempted, which may cause transactions that involve other tokens (such as batch operations) to fully revert. Consider skipping the transfer if the amount is zero, which will also save gas.

Findings

Total: 3

prize-pool/src/PrizePool.sol#L832

832:     prizeToken.safeTransfer(_to, _amount)

vault/src/Vault.sol#L1027-L1155

1027:     SafeERC20.safeTransfer(IERC20(asset()), _receiver, _assets)
...
1155:     _twabController.transfer(_from, _to, uint96(_shares))

Assessed type

Token-Transfer

c4-sponsor commented 1 year ago

asselstine marked the issue as sponsor confirmed

Picodes commented 1 year ago

Two instances are incorrect (prizeToken and twabcontroller implementations are known)

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

PierrickGT commented 1 year ago

Has been fixed, it will revert if trying to mint 0 shares. https://github.com/GenerationSoftware/pt-v5-vault/blob/0c671c910e053ab8595f05f7c62a1a9d4f8bf342/src/Vault.sol#L931