code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/utils/MultiRewardEscrow.sol#L165

Vulnerability details

Rebasing tokens make balanceOf modifications arbitrarily (e.g: Aave share tokens).

If such token is used in an escrow, the balance could become insufficient at the time of claiming rewards, making it impossible to claim rewards for that escrow.

Impact

Medium

Proof Of Concept

The claimable amount is computed, and the MultiRewardEscrow then tries to transfer it to escrow.account.

154:   function claimRewards(bytes32[] memory escrowIds) external {
155:     for (uint256 i = 0; i < escrowIds.length; i++) {
156:       bytes32 escrowId = escrowIds[i];
157:       Escrow memory escrow = escrows[escrowId];
158: 
159:       uint256 claimable = _getClaimableAmount(escrow);
160:       if (claimable == 0) revert NotClaimable(escrowId);
161: 
162:       escrows[escrowId].balance -= claimable;
163:       escrows[escrowId].lastUpdateTime = block.timestamp.safeCastTo32();
164: 
165:       escrow.token.safeTransfer(escrow.account, claimable);
166:       emit RewardsClaimed(escrow.token, escrow.account, claimable);
167:     }

If rebasing makes it so that escrow.token.balanceOf(address(this)) < claimable, the call would revert, locking escrow.token in MultiRewardEscrow.

Tools Used

Manual Analysis

Mitigation

You can add an amount parameter to claimRewards, so that users can specify how much they want to claim.

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #44

c4-sponsor commented 1 year ago

RedVeil marked the issue as sponsor confirmed