Lines of code
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L179&gt;
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L200&gt;

Vulnerability details

Impact

A hacker can drain an ERC-777 reward token funds via reentrancy. This is because in the claimRewards() function, the transfer of the reward token which triggers the hacker’s ERC-777 hook takes place before setting accruedRewards[user][_rewardTokens[i]] to zero.

Proof of Concept

The attack scenario:

Step 1. Admin adds an ERC-777 hookable token as the reward token. In such a token, a user can set a callback that will be called every time someone sends money to them.

Step 2. Hacker accumulates rewards for this token and calls claimRewards(hackerAddress, [erc777token]).
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L170&gt;

Step 3. The function reads the hacker’s reward amount from the contract’s storage:

uint256 rewardAmount = accruedRewards[user][_rewardTokens[i]];

<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L172&gt;

Step 4. The positive amount of tokens is sent to the hacker. Either on the line
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L182&gt;

_rewardTokens[i].transfer(user, rewardAmount)

or on the line
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L200&gt;

rewardToken.safeTransfer(user, payout);

Step 5. The ERC-777 hook is triggered. The hacker takes control and calls the claimRewards() function again with the same parameters.

Step 6. In the MultiRewardStaking contract, the hacker’s state has not changed, and accruedRewards[user][_rewardTokens[i]] is still equal to its original positive value. Therefore, funds are transferred to the hacker again, and the hook is triggered again, from which they can repeat the attack again.

Step 7. After the hacker repeatedly executes the reentrancy, the code flow finally reaches line 186 and sets the hacker’s accrued rewards to zero without any revert.
<https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardStaking.sol#L186&gt;

accruedRewards[user][_rewardTokens[i]] = 0;

Since no errors occur, the hacker gets a profit.

Tools Used

Manual analysis

Recommended Mitigation Steps

Follow the Check-Effect-Interaction pattern and set the state variable accruedRewards[user][_rewardTokens[i]] to zero before doing an external transfer call.

---

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

All reactions