search

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

0 stars 0 forks source link

Yield sources cannot be swapped back #51

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

The _setYieldSource function of SwappableYieldSource calls the safeApprove function to approve the yield sources with the maximum allowance of transferring underlying tokens. However, according to OpenZeppelin's implementation, the safeApprove function succeeds only if the current allowance is zero or the allowance to be set is zero (see the following link).

As a result, a yield source cannot be set twice by the contract. For example, set A -> set B -> set A is not possible since A's allowance is non-zero in the second "set A", causing the transaction to revert.

Proof of Concept

Referenced code: SwappableYieldSource.sol#L259

OpenZeppelin - SafeERC20.sol#L52-L55

Recommended Mitigation Steps

Use safeIncreaseAllowance to increase the allowance to the maximum instead.

PierrickGT commented 3 years ago

This issue has been fixed by decreasing the allowance of the old yield source when swapping for a new yield source. https://github.com/pooltogether/swappable-yield-source/pull/3