code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L309

Vulnerability details

Impact

When a user interacts with a gauge and a new balance checkpoint is created (in storage of this gauge), then checkpoint.voted for this new checkpoint is always false.

Unless users are aware of this bug and call voter.poke after each interaction with the gauge or before claiming rewards, they will be unfairly missing rewards.

Proof of Concept

User Alice votes. That creates her first checkpoint with voted == true. Then she waits some time and deposits tokens to the same gauge. This creates another checkpoint. Boolean variable voted is calculated in this line:

bool prevVoteStatus = (_nCheckPoints > 0) ? checkpoints[account][_nCheckPoints].voted

_nCheckPoints is index of a checkpoint that doesn’t yet exist, so checkpoints[account][_nCheckPoints].voted evaluates to false

Missing of rewards is caused by the following lines in earned function:

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L489-L490

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L499-L500

Tools Used

Tested in Foundry

Recommended Mitigation Steps

Change

bool prevVoteStatus = (_nCheckPoints > 0) ? checkpoints[account][_nCheckPoints].voted : false;

to

bool prevVoteStatus = (_nCheckPoints > 0) ? checkpoints[account][_nCheckPoints - 1].voted : false;

Last checkpoint has index _nCheckPoints - 1, not _nCheckPoints.

pooltypes commented 2 years ago

Duplicate of #59

GalloDaSballo commented 2 years ago

Dup of #59

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