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

User exit/claim methods should not have a whenNotPaused modifier #113

Closed
c4-bot-5 opened this issue Dec 16, 2023 · 7 comments
Closed

User exit/claim methods should not have a whenNotPaused modifier #113

c4-bot-5 opened this issue Dec 16, 2023 · 7 comments
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-1249 insufficient quality report This report is not of sufficient quality partial-75 Incomplete articulation of vulnerability; eligible for partial credit only (75%)

Comments

@c4-bot-5
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L259

Vulnerability details

Summary

The getRewards() function here https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L259 allows users to claim reward, but a call has been made to the mint function at https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/rate-limits/RateLimitedMinter.sol#L49 which has whenNotPaused modifier . This will denied user from getting rewards. This opens up an attack vector, where the protocol owner can decide if the users are able to withdraw/claim any funds from it.

This is a common centralization problem which means the contract owner can "rug" users.

Proof of Concept

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/SurplusGuildMinter.sol#L259

Tools Used

manual code review

Impact

user funds can be left stuck in the contract
funds can stay forever if a Governor or Guardian renounce ownership or compromised (The multiSig) will leave the funds there forever

Recommended Mitigation Steps

Remove the whenNotPaused modifier from mint function or implement a way where user can claim reward without minting them(pre-mint), so users can claim vested tokens even if admin pauses the contract.

Assessed type

Access Control

@c4-bot-5 c4-bot-5 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 16, 2023
c4-bot-10 added a commit that referenced this issue Dec 16, 2023
@0xSorryNotSorry
Copy link

inflated, since the Governor role is trusted

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Jan 3, 2024
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 21, 2024
@c4-judge
Copy link
Contributor

Trumpero marked the issue as unsatisfactory:
Invalid

@Trumpero
Copy link

After reviewing again, I believe this is a dup of #1249.

@c4-judge
Copy link
Contributor

Trumpero marked the issue as duplicate of #1249

@Trumpero
Copy link

Give this issue 75% partial credit due to its lack of quality

@c4-judge
Copy link
Contributor

Trumpero marked the issue as partial-75

@c4-judge c4-judge added partial-75 Incomplete articulation of vulnerability; eligible for partial credit only (75%) and removed unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Jan 31, 2024
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-1249 insufficient quality report This report is not of sufficient quality partial-75 Incomplete articulation of vulnerability; eligible for partial credit only (75%)
Projects
None yet
Development

No branches or pull requests

5 participants