Lines of code

Vulnerability details

Impact

Some ERC20 do allow for user’s control of execution. For example, ERC777 has tokensReceived() hook. This way, an ability to reenter can be executed with the usage of any such tokens.

AmmGauge stake do not control for reentrancy and uses balance difference to calculate how much to add to user’s accounted funds.

Suppose Bob has 100 tokens, runs stake and reenters 4 times, so run stake 5 times in total, with 20 token each time. His first run will recognize 100 tokens as a deposit, the second run will recognize 80, and only his last one will recognize only 20 tokens he actually sent. This way the Vault records 100 + 80 + 60 + 40 + 20 = 300 tokens for the 20 * 5 = 100 tokens Bob deposited.

This way Bob will bloat his account and finally exit with all the funds AmmGauge have.

Setting severity to high as that’s the loss of funds of all other stakers.

Proof of Concept

stakeFor allows reentrancy and determine the amount provided after pulling the funds from the user based on balance change:

<https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/tokenomics/AmmGauge.sol#L103-L113&gt;

Similarly, unstakeFor allows reentrancy and can be gamed by orchestrating the cumulative balance change:

<https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/tokenomics/AmmGauge.sol#L131&gt;

_userCheckpoint called in both cases will record zero reward balance change on nested runs, not preventing execution.

Recommended Mitigation Steps

Introduce reentrancy control, for example a well-tested nonReentrant modifier.

---

The text was updated successfully, but these errors were encountered:

All reactions