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

Approved lenders and borrowers can cause DoS of the protocol #257

Closed
code423n4 opened this issue Aug 17, 2022 · 2 comments
Closed

Approved lenders and borrowers can cause DoS of the protocol #257

code423n4 opened this issue Aug 17, 2022 · 2 comments
Labels
bug Something isn't working downgraded by judge duplicate This issue or pull request already exists edited-by-warden QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

code423n4 commented Aug 17, 2022

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPair.sol#L288-L315

Vulnerability details

Impact

In FraxlendPair is setter for approved lenders (resp. borrowers) accessible only to approved lenders (resp. borrowers). These setters allows to de/whitelist any lenders or borrowers.

This can be critical if any approved lender or borrower decides to de-whitelist all other users.

Since, this issue requires elevated privileges, it is considered as a medium risk.

Proof of Concept

Alice is an approved borrower. She will decide to de-whitelist all other borrowers, except her. As a result, Alice is the only person in the protocol that can borrow assets (call the borrowAsset function).

Steps:
#1 Call setApprovedBorrowers(, false) - (https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPair.sol#L307-L315)

Same approach applies respectively for approved lending. In that case it will cause that nobody is able to make new deposits, mints and liquidations.

Tools Used

Manual review

Recommended Mitigation Steps

Adjust the logic of setting approved borrowers and lenders.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Aug 17, 2022
code423n4 added a commit that referenced this issue Aug 17, 2022
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value edited-by-warden and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Aug 17, 2022
@code423n4 code423n4 changed the title Approved lenders and borrowers can cause DOS of the protocol Approved lenders and borrowers can cause DoS of the protocol Aug 17, 2022
@amirnader-ghazvini amirnader-ghazvini added the duplicate This issue or pull request already exists label Aug 29, 2022
@amirnader-ghazvini
Copy link
Collaborator

Duplicate of #157

@amirnader-ghazvini amirnader-ghazvini marked this as a duplicate of #157 Aug 29, 2022
@gititGoro gititGoro added downgraded by judge QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Sep 27, 2022
@gititGoro
Copy link
Collaborator

Duplicate of #253

@gititGoro gititGoro marked this as a duplicate of #253 Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge duplicate This issue or pull request already exists edited-by-warden QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

3 participants