code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L83-L90 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L327

Vulnerability details

Impact

DoS issues when users try to claim rewards.

Proof of Concept

tokenRewardsPerEpoch[token][adjustedTstamp] for a given epoch only increases when bribe.notifyRewardAmount is called and never decreases. Before it's called first time in a given epoch, this value should be equal to all fees accumulated in the bribe.

After bribe.deliverReward(token, epochStart) is called first time in the epoch, full balance of this token stored in the bribe should be transferred out to the gauge.

When this function is called 2nd time, it'll try to transfer tokenRewardsPerEpoch[token][adjustedTstamp] to gauge again, but it'll fail because the balance is already 0.

That is, unless someone has sent reward tokens to the bribe directly without notifyRewardAmount, or unless adjustedTstamp in notifyRewardAmount points to later epoch. But then we'd be transferring these tokens too early and there will be an DoS issue in next epoch.

This function is called when voter.distribute is called and voter.distribute is called every time gauge rewards are claimed (gauge.getReward).

After maybe sending some reward tokens with voter.notifyRewardAmount to increase voter.claimable to pass

if (_claimable > IGauge(_gauge).left(base) && _claimable / DURATION > 0)

in voter.distribute, each attempt to claim rewards by users will fail because deliverBribes will revert.

I haven't tested it properly in foundry because 5 minutes until deadline but it should work.

Tools Used

Manual analysis

Recommended Mitigation Steps

Decrease tokenRewardsPerEpoch[token][adjustedTstamp] after transferring tokens from the bribe.

pooltypes commented 2 years ago

Duplicate of #141

GalloDaSballo commented 2 years ago

Dup of #141

code-423n4 / 2022-05-velodrome-findings