code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L257-L264 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L372

Vulnerability details

Impact

In function _checkpoint(), new values of userPointHistory and pointHistory are override old values instead of appending to the end of the list, i.e creating new element.

The result is if we try to get balanceOf or totalSupply at current block number, it just return wrong value because values of globalEpoch is overrided.

Proof of Concept

Line 257-264

if (uEpoch == 0) {
    userPointHistory[_addr][uEpoch + 1] = userOldPoint;
}

userPointEpoch[_addr] = uEpoch + 1;
userNewPoint.ts = block.timestamp;
userNewPoint.blk = block.number;
userPointHistory[_addr][uEpoch + 1] = userNewPoint;

When uEpoch == 0, values of userPointHistory with index = uEpoch + 1 is updated to userOldPoint but in line 264, values of userPointHistory with index = uEpoch + 1 is overrided to userNewPoint which basically makes line 257-259 has no meaning.

Similarly issue with value of pointHistory[epoch] in line 372

Tools Used

Manual Review

Recommended Mitigation Steps

Update logic of _checkpoint(), for example, use ++ operator to make sure epoch is increased each time it appends new element.

lacoop6tu commented 2 years ago

Lines 257-259 are actually meaningless and are not causing any error/side effects, they can be removed. in mStable impl, they used a dynamic array so they had to push an empty point with the first index, while we are using a fixed one which is already there. But i don't see the similarity for "Similarly issue with value of pointHistory[epoch] in line 372"

elnilz commented 2 years ago

so essentially the issue is just that lines 257-259 are unnecessary but don't affect in any way the proper functioning of the system. that's why I think its a QA severity

gititGoro commented 2 years ago

Duplicate of #316

code-423n4 / 2022-08-fiatdao-findings