Lines of code

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L1209 https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L1157-L1162

Vulnerability details

Impact

The repayAssetWithCollateral() function is difficult to use in order to pay off a user's entire balance. In an effort to pay off the user's entire debt, they will attempt to calculate the amount of collateral necessary that equivalates to their debt shares. If the amount of asset received via the swap is larger than their debt position, execution will revert due to underflow.

Proof of Concept

The repayAssetWithCollateral() function takes _collateralToSwap as a parameter.

    function repayAssetWithCollateral(
        address _swapperAddress,
        uint256 _collateralToSwap,
        uint256 _amountAssetOutMin,
        address[] calldata _path
    ) external nonReentrant isSolvent(msg.sender) returns (uint256 _amountAssetOut) {

This amount is used to swap to the asset token. The asset balance is determined via the contract balance difference pre and post swap:

        uint256 _initialAssetBalance = _assetContract.balanceOf(address(this));
        ISwapper(_swapperAddress).swapExactTokensForTokens(
            _collateralToSwap,
            _amountAssetOutMin,
            _path,
            address(this),
            block.timestamp
        );
        uint256 _finalAssetBalance = _assetContract.balanceOf(address(this));

        // Note: VIOLATES CHECKS-EFFECTS-INTERACTION pattern, make sure function is NONREENTRANT
        // Effects: bookkeeping
        _amountAssetOut = _finalAssetBalance - _initialAssetBalance;

Shares are calculated and a repayment is attempted:

        uint256 _sharesToRepay = _totalBorrow.toShares(_amountAssetOut, false);
        ...
        _repayAsset(_totalBorrow, _amountAssetOut.toUint128(), _sharesToRepay.toUint128(), address(this), msg.sender);

In the _repayAsset() function, if too many shares are attempted for repay, execution will revert due to underflow.

    function _repayAsset(
        VaultAccount memory _totalBorrow,
        uint128 _amountToRepay,
        uint128 _shares,
        address _payer,
        address _borrower
    ) internal {
        ...
        userBorrowShares[_borrower] -= _shares;

Tools Used

Manual review.

Recommended Mitigation Steps

This issue also exists in the normal repayment flow, though the user is able to provide the exact share value to repay. A fix for this issue would involve refunding the amount of asset that exceeds the debt shares.

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