Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent delegation to zero #88

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Prevent delegation to zero #88

merged 1 commit into from
Apr 18, 2024

Conversation

ryangoree
Copy link
Member

@ryangoree ryangoree commented Apr 18, 2024

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.

@ryangoree ryangoree merged commit 663f32f into main Apr 18, 2024
3 checks passed
@ryangoree ryangoree deleted the ryan-patches branch April 18, 2024 15:37
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8740746981

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 94.069%

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

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants