Lines of code

https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPairCore.sol#L739-L758 https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPairCore.sol#L707-L731

Vulnerability details

Impact

The borrowAsset function can be called with the _collateralAmount input being set to 0. This will successfully bypass the step for providing any collateral tokens while directly borrowing the asset tokens. When a borrower borrows assets without providing any collaterals, she or he has nothing to lose so there is no incentive to return the assets in the future. As a result, the lender will lose all the asset tokens that are lent out.

Proof of Concept

The following steps can occur.

For any deployed pair, the borrower calls the following borrowAsset function in the FraxlendPairCore contract with the _collateralAmount input being set to 0.

https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPairCore.sol#L739-L758

    function borrowAsset(
        uint256 _borrowAmount,
        uint256 _collateralAmount,
        address _receiver
    )
        external
        isNotPastMaturity
        whenNotPaused
        nonReentrant
        isSolvent(msg.sender)
        approvedBorrower
        returns (uint256 _shares)
    {
        _addInterest();
        _updateExchangeRate();
        if (_collateralAmount > 0) {
            _addCollateral(msg.sender, _collateralAmount, msg.sender);
        }
        _shares = _borrowAsset(_borrowAmount.toUint128(), _receiver);
    }

Calling borrowAsset evaluates the (_collateralAmount > 0) if condition, which is false because _collateralAmount is set to 0 in Step 1. This means that _addCollateral(msg.sender, _collateralAmount, msg.sender); will not be run. Thus, the step for providing collateral tokens to the pair is successfully bypassed.
Calling borrowAsset further executes the following _borrowAsset function with the specified _borrowAmount input for how much to borrow and _receiver input being the borrower's own address.

https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPairCore.sol#L707-L731

    function _borrowAsset(uint128 _borrowAmount, address _receiver) internal returns (uint256 _sharesAdded) {
        VaultAccount memory _totalBorrow = totalBorrow;

        // Check available capital
        uint256 _assetsAvailable = _totalAssetAvailable(totalAsset, _totalBorrow);
        if (_assetsAvailable < _borrowAmount) {
            revert InsufficientAssetsInContract(_assetsAvailable, _borrowAmount);
        }

        // Effects: Bookkeeping to add shares & amounts to total Borrow accounting
        _sharesAdded = _totalBorrow.toShares(_borrowAmount, true);
        _totalBorrow.amount += _borrowAmount;
        _totalBorrow.shares += uint128(_sharesAdded);
        // NOTE: we can safely cast here because shares are always less than amount and _borrowAmount is uint128

        // Effects: write back to storage
        totalBorrow = _totalBorrow;
        userBorrowShares[msg.sender] += _sharesAdded;

        // Interactions
        if (_receiver != address(this)) {
            assetContract.safeTransfer(_receiver, _borrowAmount);
        }
        emit BorrowAsset(msg.sender, _receiver, _borrowAmount, _sharesAdded);
    }

As long as _borrowAmount is not more than the available asset token amount, calling borrowAsset will not revert with the InsufficientAssetsInContract error.
After the bookkeeping is done for updating totalBorrow.amount, totalBorrow.shares, and userBorrowShares[msg.sender], assetContract.safeTransfer(_receiver, _borrowAmount); is executed because the (_receiver != address(this)) if condition would be evaluated to true as _receiver is the borrower's own address.
Now, as the borrower receives _borrowAmount of the asset tokens without providing any collateral tokens, she or he risks nothing for the loan. The borrower has no incentive to return the borrowed asset tokens in the future, and the lender will lose all the asset tokens that are lent out consequently.

Tools Used

VSCode

Recommended Mitigation Steps

https://github.com/code-423n4/2022-08-frax/blob/main/src/contracts/FraxlendPairCore.sol#L739-L758 can be changed to the following code. This code now requires that some collateral tokens must be provided by adding the require statement. Also, the _addCollateral function will always run for providing _collateralAmount of the collateral tokens to the pair.

    function borrowAsset(
        uint256 _borrowAmount,
        uint256 _collateralAmount,
        address _receiver
    )
        external
        isNotPastMaturity
        whenNotPaused
        nonReentrant
        isSolvent(msg.sender)
        approvedBorrower
        returns (uint256 _shares)
    {
        require(_collateralAmount > 0, "no collateral provided");

        _addInterest();
        _updateExchangeRate();
        _addCollateral(msg.sender, _collateralAmount, msg.sender);
        _shares = _borrowAsset(_borrowAmount.toUint128(), _receiver);
    }

Moreover, this borrowAsset function should be updated further to check that _borrowAmount of the asset tokens and _collateralAmount of the collateral tokens are in an allowed relationship according to the LTV setting, exchange rate etc. This check is not yet added in the function above.

code-423n4 / 2022-08-frax-findings