Open code423n4 opened 2 years ago
As most of wardens reported in duplicated, this is Medium finding 2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
Lines of code
https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L509-L515
Vulnerability details
[PNM-001]
increaseUnlockTime
missing_checkpoint
for delegated values.Links
Description
In the VotingEscrow contract, users can increase their voting power by:
Specifically, when users are delegated by other users through the
delegate
function, the delegated user gains control over the delegate funds from the delegating user.The delegated user can further increase this power by increasing the time that the delegated funds are locked by calling
increaseUnlockTime
, resulting in ALL the delegated funds controlled by the delegated user, including those that do not originate from the delegated user, being used to increase the voting power of the user.The issue lies in the following scenario: If user A delegates to user B, and then user B delegates to user C, user B loses the ability to extend his or her voting power by
increaseUnlockTime
due to a missing_checkpoint
operation. If user B calls theincreaseUnlockTime
function, the_checkpoint
operation will not proceed, as user B is delegating to user C. However, B still owns delegated funds, in the form of the funds delegated from user A. Therefore, user B should still gain voting power fromincreaseUnlockTime
, even though user B is delegating.PoC / Attack Scenario
Assume three users, Alice, Bob, and Carol, who each possess
locks
with 10 units ofdelegate
value. Also assume that the unlock time is 1 week.delegate
, value, Bob has 10delegate
value, and Carol has 20delegate
value.increaseUnlockTime
to 2 weeks, resulting in_checkpoint
raising her voting power accordingly.increaseUnlockTime
to 2 weeks, resulting in no change in his voting power, even though he has 10 units ofdelegate
value.Suggested Fix
Move the
_checkpoint
outside of theif
statement on line 514.