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

Whitelist lender can prevent liquidation by removing all other lenders from whitelist #80

Open
code423n4 opened this issue Aug 15, 2022 · 1 comment
Labels
bug Something isn't working downgraded by judge QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L911-L942
https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L950-L1032

Vulnerability details

Impact

Lender blocks liquidations

Proof of Concept

FraxlendPair.sol#setApprovedLenders can be called by any whitelisted lender to both revoke and add approved lenders to the whitelist. A malicious lender could remove every other whitelisted lender from the pair then refuse to liquidate the position themselves, making it impossible to liquidate.

Exploiting this is straight forward. If the lending market is only whitelisted for lenders, the malicious lender could withdraw their lending position, deposit collateral and borrow everything else in the pool then remove all other lenders from the pool. Something like this could be used to open a high leverage zero risk position.

If the market has whitelist borrowers, the lender could easily collude with a borrower to pull off the attack.

Tools Used

Recommended Mitigation Steps

I don't see why FraxlendPairCore.sol#liquidate and liquidateClean can only be called by whitelist lenders. It is in the best interest of both parties if anyone can liquidate a position even in a whitelisted market. If it truly is desirable that only whitelist lenders are able to liquidate, then FraxlendPair.sol#setApprovedLenders should be modified to only allow lenders to add new lenders rather than remove current lenders.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Aug 15, 2022
code423n4 added a commit that referenced this issue Aug 15, 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 3 (High Risk) Assets can be stolen/lost/compromised directly labels Sep 27, 2022
@gititGoro gititGoro removed the duplicate This issue or pull request already exists label Oct 11, 2022
@gititGoro gititGoro reopened this Oct 11, 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 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