# Fraxlend PoC

## Summary

High severity vulnurability was found in Fraxlend Whitelist.

Where - at FraxlendPair, at permissioned ones (with `borrowerWhitelistActive=True` and `lenderWhitelistActive=True`)

Finding:
1) Any whitelisted borrower can add new whitelisted borrowers and ban existing whitelisted borrowers
2) Any whitelisted lender can add new whitelisted lender and ban existing whitelisted lender

We found this behavior strange and likely not designed.

## Contracts used for description
- Factory
https://etherscan.io/address/0x5d6e79bcf90140585ce88c7119b7e43caaa67044#code
- Pair
https://etherscan.io/address/0x794F6B13FBd7EB7ef10d1ED205c9a416910207Ff#code

## Detailed Description

All Pairs have modifiers to check the allowance for an address to lend/borrow.
#### modifier `approvedBorrower` is used in functions:
- `borrowAsset()`
- `leveragedPosition()`
- `setApprovedBorrowers()` (!)
#### modifier `approvedLender` is used in functions:
- `deposit()`
- `liquidate()`
- `setApprovedLenders()` (!)

`setApprovedBorrowers()` and `setApprovedLenders()` set values in the mappings `approvedBorrowers` and `approvedLenders`.

Example `setApprovedLenders()`:

function setApprovedLenders(address[] calldata _lenders, bool _approval) external approvedLender(msg.sender) {
        for (uint256 i = 0; i < _lenders.length; i++) {
            // Do not set when _approval == false and _lender == msg.sender
            if (_approval || _lenders[i] != msg.sender) {
                approvedLenders[_lenders[i]] = _approval;
                emit SetApprovedLender(_lenders[i], _approval);
            }
        }
    }

#### As you can see Borrowers are allowed to set the list of allowed borrowers, and Lenders are allowed to set the list of allowed Lenders.

This strange pattern works different in two types of Fraxlend Pairs.
**The vulnurability exists only in Permissioned Pairs**.
So lets take a look at two types of Pairs.

#### 1) Permisionless, where anyone can lend/borrow
All current Fraxlend pairs with liquidity are permissionless. They are created via Factory.deploy() function that sets both `borrowerWhitelistActive` and `lenderWhitelistActive` as `false`, by default. This setting is immutable for a Pair.
As Pairs are permissionless, any address is ok for modifiers `approvedBorrower` and `approvedLender`. Thus, anyone can call functions `setApprovedBorrowers()` and `setApprovedLenders()` and manage mappings `approvedBorrowers` and `approvedLenders`. That is strange, but **is not dangerous** - as a Pair still remains permisionless.

#### 2) Permissioned, where a specified list of addressed is allowed to lend/borrow
Fraxlend does not have these Pairs operating for now. But they are designed according to Docs.
https://docs.frax.finance/fraxlend/key-concepts

Custom Term Sheet deployments can set this value manually, 
even creating under-collateralized loans 
by setting the value above 100%.  
Maximum LTV configured above 100% 
must be accompanied by a borrower whitelist 
to protect against malicious actors. 

It means that Permissioned Pairs are possible, likely for Pairs with LTV > 100%.

These pairs can be created using `Factory.deployCustom()`, with `_approvedBorrowers` and `_approvedLenders` as arguments. Is must be noted that these permissioned Pairs can only be deployed by the allowed list of deployers, stored here.
https://etherscan.io/address/0x118C1462AA28bF2ea304f78f49C3388cfd93234e#code

When a permissioned Pair is created modifiers `approvedBorrower` and `approvedLender` check these things:
Example `approvedBorrower`

modifier approvedBorrower() {
        if (borrowerWhitelistActive && !approvedBorrowers[msg.sender]) {
            revert OnlyApprovedBorrowers();
        }
        _;
    }

It means that for a permissioned Pair a contract check that callers are in mappings `approvedBorrowers` or `approvedLender`.
So anyone in `approvedBorrowers` or `approvedLenders` mappings pass modifiers `approvedBorrower` and `approvedLender`.
Therefore, they can call `setApprovedBorrowers()` and `setApprovedLenders()` in order to manage the storage of mappings `approvedBorrowers` or `approvedLender`.

Any whitelisted address can set new address as allowed and/or ban existing addressed.
This is a strange behavior and we expect it was not designed.

## Steps to reproduce

1. Prerequsites: wait for liquid Permissioned Pair to be deployed
2. Whitelisted borrower calls `setApprovedBorrowers()`
(OR) Whitelisted lender calls `setApprovedLenders()`
Callers can input any list of addresses and their status (allowed / not allowed). With one exception - a caller cannot change own status.

## How it is managed now (probably)

The description of permissioned pairs states that it will avoid `malicious actors` and they are likely will be KYC. Thus, we expect the team's assumptions that any bad actor can be sactioned legally, offchain.

## To sum up

We think that this behavior for a permisioned pairs does not provide useful features, but introduce risks - that any allowed user is like an owner of a contract with too much privelleges.

We recommend:
- removing this feature
- making the owner the only role to add and ban addresses manually