delvtech / council

Flexible DAO governance smart contracts, developed by Delv.
Apache License 2.0
93 stars 19 forks source link

Prevent delegation to zero #88

Closed ryangoree closed 6 months ago

ryangoree commented 6 months ago

This PR fixes a permanent freezing of funds vulnerability in the Locking Vault by preventing delegation to address(0) in the changeDelegation function. This is required because the changeDelegation function will reduce the voting power of the user's current delegate before adding it to the new delegate, but the deposit function treats delegation to address(0) as a first time deposit and will only increase the voting power of the firstDelegation by the newly deposited amount, without re-assigning existing voting power.

As an example:

  1. Victim has a deposit of 100,000 ELFI and has delegated to themselves.
    • first time deposit so Victim.delegate = Victim, Victim.amount += 100,000
  2. Attacker deposits 10,000 ELFI by delegating to themselves.
    • first time deposit so Attacker.delegate = Attacker, Attacker.amount += 10,000
  3. Attacker changes delegation to address(0).
    • Attacker.amount -= 10,000, Attacker.delegate = address(0), address(0).amount += 10,000
  4. Attacker deposits 0 and delegates to the victim, this will set the delegation to victim but not add any voting power.
    • treated as first time deposit so Attacker.delegate = Victim, Victim.amount += 0
  5. Attacker changes delegation to address(0), this backs out 10,000 ELFI voting power and results in only 90,000 ELFI left.
    • Victim.amount -= 10,000, Attacker.delegate = address(0), address(0).amount += 10,000
  6. Repeat step 4-5 until the victim has 0 voting power.
  7. Victim is now unable to withdraw or change delegation.
coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 8740746981

Details


Totals Coverage Status
Change from base Build 4878764490: -0.1%
Covered Lines: 512
Relevant Lines: 529

💛 - Coveralls