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

Can avoid guild slashing by frontrunning notifyGaugeLoss with decrementGauge #310

Closed
c4-bot-10 opened this issue Dec 21, 2023 · 6 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-877 sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-10
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L123

Vulnerability details

Impact

When a gauge loss occurs, the protocol intends to lock the gauge weight voting for that gauge and slash it, but it's possible to frontrun the gauge loss by decrementing your gauge weight, thereby avoiding being slashed.

Proof of Concept

When a gauge incurs a loss, the ProfitManager notifies the GuildToken by calling notifyGaugeLoss, which in turn locks all the current gauge weight.

require(
    _lastGaugeLossApplied >= _lastGaugeLoss,
    "GuildToken: pending loss"
);

The intention for this as stated in the documentation is to prevent bank runs by disallowing users from withdrawing before being liquidated. However, this doesn't actually accomplish this intention as users can simply watch for the notifyGaugeLoss call in the mempool and have a script frontrun this call to decrement their gauge weight. This significantly reduces the risk incurred by gauge voting which thus applies that risk to the protocol instead.

Tools Used

  • Manual review

Recommended Mitigation Steps

If a locking mechanism is intended to be used to prevent bank runs, the position should be locked upon deposit for a pre-defined duration. One option would be to incorporate the locking duration into the gauge weight such that locking for a longer duration results in a greater gauge weight.

Assessed type

Timing

@c4-bot-10 c4-bot-10 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 21, 2023
c4-bot-1 added a commit that referenced this issue Dec 21, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Dec 29, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #906

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #877

@c4-judge
Copy link
Contributor

Trumpero marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 25, 2024
@c4-judge
Copy link
Contributor

Trumpero marked the issue as unsatisfactory:
Invalid

1 similar comment
@c4-judge
Copy link
Contributor

Trumpero marked the issue as unsatisfactory:
Invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-877 sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants