An attacker wishing to harm users could perform the following attack:
The attacker listens for transactions on mempool that are calling supplyTokenTo.
When he listen the victim (or victims), the attacker sends X aTokens to the contract and cause a re-order (e.g. by bribing a miner) to guarantee that the transfer of aToken is the first transaction in the block.
Then one of the following damaging situations can occur:
If X is large enough, _shares will be 0 and will cause a revert in L233. This prevents the user(s) of adding tokens to the pool.
If X is not large enough to cause a revert, it can also cause a damage since the attacker decreases the expected shares that the user will receive. Indeed, when the user try to redeem the same tokens using redeemToken he may get blocked since he won’t have sufficient token to burn and L256 will revert.
Impact: The attacker can target just a subset of users, but with enough capital can degrade the experience of all users that are trying to supply tokens to Pool Together.
Some tokens require the approval to be zero before calling approve with a non-zero value
Summary: Some ERC20 tokens require approving to 0 first before approving a new value.
Details: Some tokens (such as USDT) do not work when changing the allowance from an existing non-zero allowance value.
Impact: It can break the contract composability if _tokenAddress() correspond to address of such tokens (e.g. USDT). Otherwise, it won’t cause any issue.
gititGoro
commented
2 years ago
Originally part of a QA report by Tadashi https://github.com/code-423n4/2022-04-pooltogether-findings/issues/69
Griefing attack by transferring aTokens to the contract
Summary:
_tokenToSharesuses the following formula for computing the total shares of an user:An attacker wishing to harm users could perform the following attack:
supplyTokenTo.aTokens to the contract and cause a re-order (e.g. by bribing a miner) to guarantee that the transfer ofaTokenis the first transaction in the block.Then one of the following damaging situations can occur:
_shareswill be 0 and will cause a revert in L233. This prevents the user(s) of adding tokens to the pool.redeemTokenhe may get blocked since he won’t have sufficient token to burn and L256 will revert.Impact: The attacker can target just a subset of users, but with enough capital can degrade the experience of all users that are trying to supply tokens to Pool Together.
Some tokens require the approval to be zero before calling approve with a non-zero value
Summary: Some ERC20 tokens require approving to 0 first before approving a new value.
Details: Some tokens (such as USDT) do not work when changing the allowance from an existing non-zero allowance value.
Mitigation: Change L182 to
Impact: It can break the contract composability if
_tokenAddress()correspond to address of such tokens (e.g. USDT). Otherwise, it won’t cause any issue.