Impossible to `setCreationCode()` with code size less than 13K

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairDeployer.sol#L170-L178 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairDeployer.sol#L207-L210

Vulnerability details

In case of setting the creation code to less than 13K the creation would fail at 2 points:

• BytesLib.slice(_creationCode, 0, 13000) would fail since slice() requires that _bytes.length would be at least start + length.
• SSTORE2.read(contractAddress2) at _deployFirst() would fail since calling this on address without code would cause a math underflow (due to pointer.code.length - DATA_OFFSET)

Impact

In case the code of the pair gets smaller (e.g. optimization, moving part of the code to a library etc.) to 13K or less, it'd be impossible to set it as the new creation code (or in the case of the 2nd issue, it'd be impossible to deploy it)

Proof of Concept

In the following test I try to set creation code to a smaller mock pair. The test was added to src/test/e2e/FraxlendPairDeployerTest.sol:

    function testChangeCreationCodeBug() public {
        // Setup contracts
        setExternalContracts();
        startHoax(COMPTROLLER_ADDRESS);
        setWhitelistTrue();
        vm.stopPrank();
        console2.log("Mock pair size:", type(MockPair).creationCode.length);

        vm.startPrank(deployer.owner());
        deployer.setCreationCode(type(MockPair).creationCode);

        // Set initial oracle prices
        bytes memory _rateInitData = defaultRateInitForLinear();
        deployer.deploy(
            abi.encode(
                address(asset),
                address(collateral),
                address(oracleMultiply),
                address(oracleDivide),
                1e10,
                address(linearRateContract),
                _rateInitData
            )
        );

    }

The mock pair was created as src/contracts/audit/MockPair.sol:

// SPDX-License-Identifier: ISC
pragma solidity ^0.8.15;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/security/Pausable.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/utils/math/SafeCast.sol";
import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "../FraxlendPairConstants.sol";
import "../libraries/VaultAccount.sol";
import "../libraries/SafeERC20.sol";
import "../interfaces/IERC4626.sol";
import "../interfaces/IFraxlendWhitelist.sol";
import "../interfaces/IRateCalculator.sol";
import "../interfaces/ISwapper.sol";

/// @title FraxlendPairCore
/// @author Drake Evans (Frax Finance) https://github.com/drakeevans
/// @notice  An abstract contract which contains the core logic and storage for the FraxlendPair
contract MockPair is FraxlendPairConstants, ERC20, Ownable, Pausable, ReentrancyGuard {
    using VaultAccountingLibrary for VaultAccount;
    using SafeERC20 for IERC20;
    using SafeCast for uint256;

    string public version = "1.0.0";

    // ============================================================================================
    // Settings set by constructor() & initialize()
    // ============================================================================================

    // Asset and collateral contracts
    IERC20 internal immutable assetContract;
    IERC20 public immutable collateralContract;

    // Oracle wrapper contract and oracleData
    address public immutable oracleMultiply;
    address public immutable oracleDivide;
    uint256 public immutable oracleNormalization;

    // LTV Settings
    uint256 public immutable maxLTV;

    // Liquidation Fee
    uint256 public immutable cleanLiquidationFee;
    uint256 public immutable dirtyLiquidationFee;

    // Interest Rate Calculator Contract
    IRateCalculator public immutable rateContract; // For complex rate calculations
    bytes public rateInitCallData; // Optional extra data from init function to be passed to rate calculator

    // Swapper
    mapping(address => bool) public swappers; // approved swapper addresses

    // Deployer
    address public immutable DEPLOYER_ADDRESS;

    // Admin contracts
    address public immutable CIRCUIT_BREAKER_ADDRESS;
    address public immutable COMPTROLLER_ADDRESS;
    address public TIME_LOCK_ADDRESS;

    // Dependencies
    address public immutable FRAXLEND_WHITELIST_ADDRESS;

    // ERC20 token name, accessible via name()
    string internal nameOfContract;

    // Maturity Date & Penalty Interest Rate (per Sec)
    uint256 public immutable maturityDate;
    uint256 public immutable penaltyRate;

    // ============================================================================================
    // Storage
    // ============================================================================================

    /// @notice Stores information about the current interest rate
    /// @dev struct is packed to reduce SLOADs. feeToProtocolRate is 1e5 precision, ratePerSec is 1e18 precision
    CurrentRateInfo public currentRateInfo;
    struct CurrentRateInfo {
        uint64 lastBlock;
        uint64 feeToProtocolRate; // Fee amount 1e5 precision
        uint64 lastTimestamp;
        uint64 ratePerSec;
    }

    /// @notice Stores information about the current exchange rate. Collateral:Asset ratio
    /// @dev Struct packed to save SLOADs. Amount of Collateral Token to buy 1e18 Asset Token
    ExchangeRateInfo public exchangeRateInfo;
    struct ExchangeRateInfo {
        uint32 lastTimestamp;
        uint224 exchangeRate; // collateral:asset ratio. i.e. how much collateral to buy 1e18 asset
    }

    // Contract Level Accounting
    VaultAccount public totalAsset; // amount = total amount of assets, shares = total shares outstanding
    VaultAccount public totalBorrow; // amount = total borrow amount with interest accrued, shares = total shares outstanding
    uint256 public totalCollateral; // total amount of collateral in contract

    // User Level Accounting
    /// @notice Stores the balance of collateral for each user
    mapping(address => uint256) public userCollateralBalance; // amount of collateral each user is backed
    /// @notice Stores the balance of borrow shares for each user
    mapping(address => uint256) public userBorrowShares; // represents the shares held by individuals
    // NOTE: user shares of assets are represented as ERC-20 tokens and accessible via balanceOf()

    // Internal Whitelists
    bool public immutable borrowerWhitelistActive;
    mapping(address => bool) public approvedBorrowers;

    bool public immutable lenderWhitelistActive;
    mapping(address => bool) public approvedLenders;
    event Fine(uint256 line);
    // ============================================================================================
    // Initialize
    // ============================================================================================

    /// @notice The ```constructor``` function is called on deployment
    /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
    /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
    /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
    /// @param _maturityDate The maturityDate date of the Pair
    /// @param _penaltyRate The interest rate after maturity date
    /// @param _isBorrowerWhitelistActive Enables borrower whitelist
    /// @param _isLenderWhitelistActive Enables lender whitelist
    constructor(
        bytes memory _configData,
        bytes memory _immutables,
        uint256 _maxLTV,
        uint256 _liquidationFee,
        uint256 _maturityDate,
        uint256 _penaltyRate,
        bool _isBorrowerWhitelistActive,
        bool _isLenderWhitelistActive
    )  ERC20("", "") {
        // Handle Immutables Configuration
        {
            (
                address _circuitBreaker,
                address _comptrollerAddress,
                address _timeLockAddress,
                address _fraxlendWhitelistAddress
            ) = abi.decode(_immutables, (address, address, address, address));

            // Deployer contract
            DEPLOYER_ADDRESS = msg.sender;
            CIRCUIT_BREAKER_ADDRESS = _circuitBreaker;
            COMPTROLLER_ADDRESS = _comptrollerAddress;
            TIME_LOCK_ADDRESS = _timeLockAddress;
            FRAXLEND_WHITELIST_ADDRESS = _fraxlendWhitelistAddress;
        }
        emit Fine(177);
        {
            (
                address _asset,
                address _collateral,
                address _oracleMultiply,
                address _oracleDivide,
                uint256 _oracleNormalization,
                address _rateContract,

            ) = abi.decode(_configData, (address, address, address, address, uint256, address, bytes));

            // Pair Settings
            assetContract = IERC20(_asset);
            collateralContract = IERC20(_collateral);
            currentRateInfo.feeToProtocolRate = DEFAULT_PROTOCOL_FEE;
            cleanLiquidationFee = _liquidationFee;
            dirtyLiquidationFee = (_liquidationFee * 9000) / LIQ_PRECISION; // 90% of clean fee

            if (_maxLTV >= LTV_PRECISION && !_isBorrowerWhitelistActive) revert BorrowerWhitelistRequired();
            maxLTV = _maxLTV;

            // Swapper Settings
            swappers[FRAXSWAP_ROUTER_ADDRESS] = true;

            // Oracle Settings
            {
                IFraxlendWhitelist _fraxlendWhitelist = IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS);
                // Check that oracles are on the whitelist
                if (_oracleMultiply != address(0) && !_fraxlendWhitelist.oracleContractWhitelist(_oracleMultiply)) {
                    revert NotOnWhitelist(_oracleMultiply);
                }

                if (_oracleDivide != address(0) && !_fraxlendWhitelist.oracleContractWhitelist(_oracleDivide)) {
                    revert NotOnWhitelist(_oracleDivide);
                }

                // Write oracleData to storage
                oracleMultiply = _oracleMultiply;
                oracleDivide = _oracleDivide;
                oracleNormalization = _oracleNormalization;

                // Rate Settings
                if (!_fraxlendWhitelist.rateContractWhitelist(_rateContract)) {
                    revert NotOnWhitelist(_rateContract);
                }
            }

            rateContract = IRateCalculator(_rateContract);
        }
        emit Fine(177);

        // Set approved borrowers whitelist
        borrowerWhitelistActive = _isBorrowerWhitelistActive;

        // Set approved lenders whitlist active
        lenderWhitelistActive = _isLenderWhitelistActive;

        // Set maturity date & penalty interest rate
        maturityDate = _maturityDate;
        penaltyRate = _penaltyRate;
    }

    function initialize(
        string calldata _name,
        address[] calldata _approvedBorrowers,
        address[] calldata _approvedLenders,
        bytes calldata _rateInitCallData
    ) external {
    }

}

Recommended Mitigation Steps

If creation code length is 13K or less:

• Don't run BytesLib.slice()
• at deployment read only from the first address

[DrakeEvans]

Disagree with severity, in this case no pairs could ever be used because bytecode deployment would fail. No users would be harmed.

[gititGoro]

The medium risk label isn't just for leaked value. "Assets not at direct risk, but function/availability of the protocol could be impacted or leak value"

Given that 'function/availability of the protocol could be impacted', maintaining label.

Still, it's a nice workaround for the draconian eip-160.