Handle

hyh

Vulnerability details

Impact

Reward tokens accidently sent to the Stream contract cannot be recovered with recoverTokens if some reward tokens were already claimed with claimReward.
As recoverTokens is the only recovering functionality in the contract the corresponding reward tokens will be frozen.

Proof of Concept

claimReward doesn’t change rewardTokenAmount as other user’s rewards are calculated based off it.
But reward token balance of the Stream contract does change with each reward payoff:
<https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L575&gt;

This way reward tokens already claimed aren’t accounted for in recoverTokens as only rewardTokenAmount is used there:
<https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L672&gt;

It looks like no fund stealing is possible here, the vulnerability only allows for freezing of the reward tokens accidently sent to the contract.

Recommended Mitigation Steps

Add claimedRewardTokens variable tracking cumulative reward tokens claimed

Now:

uint112 private rewardTokenAmount;
...
function claimReward() public lock {
	...
	uint256 rewardAmt = ts.rewards;
	...
	ERC20(rewardToken).safeTransfer(msg.sender, rewardAmt);

	emit RewardsClaimed(msg.sender, rewardAmt);
}
...
function recoverTokens(address token, address recipient) public lock {
...
	if (token == rewardToken) {
		...
		uint256 excess = ERC20(token).balanceOf(address(this)) - (rewardTokenAmount + rewardTokenFeeAmount);
		ERC20(token).safeTransfer(recipient, excess);
...

To be:

uint112 private rewardTokenAmount;
uint112 private claimedRewardTokens;
...
function claimReward() public lock {
	...
	uint256 rewardAmt = ts.rewards;
	...
	claimedRewardTokens += rewardAmt;
	ERC20(rewardToken).safeTransfer(msg.sender, rewardAmt);

	emit RewardsClaimed(msg.sender, rewardAmt);
}
...
function recoverTokens(address token, address recipient) public lock {
...
	if (token == rewardToken) {
		...
		uint256 excess = ERC20(token).balanceOf(address(this)) - (rewardTokenAmount - claimedRewardTokens + rewardTokenFeeAmount);
		ERC20(token).safeTransfer(recipient, excess);
...

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

All reactions