sherlock-audit / 2024-01-napier-judging

9 stars 6 forks source link

Atharv - Improper Validation of Create2 Return Value #9

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Atharv

medium

Improper Validation of Create2 Return Value

Summary

No checking whether the code is deployed on that address or not.

Vulnerability Detail

In TranchFactory.sol we have function deployTranche() to deploy the tranches using create2 from Create2TrancheLib library. The function does not revert properly if there is a failed contract deployment or revert from the create2 opcode as it does not properly check the returned address for bytecode. The create2 opcode returns the expected address which will never be the zero address and equal to the computed address(as is what is currently checked here).

function deployTranche(
        address adapter,
        uint256 maturity,
        uint256 tilt,
        uint256 issuanceFee
    ) external returns (address) {
        if (msg.sender != management) revert OnlyManagement();
        if (adapter == address(0)) revert ZeroAddress();
        if (tranches[adapter][maturity] != address(0)) revert TrancheAlreadyExists();
        if (maturity <= block.timestamp) revert MaturityInvalid();
        if (tilt >= MAX_BPS) revert TiltTooHigh(); // tilt: [0, 100%) exclusive
        if (issuanceFee > MAX_ISSUANCE_FEE_BPS) revert IssueanceFeeTooHigh(); // issuanceFee: [0, 5%] inclusive

        // NOTE: computed Tranche address is used before deployed. it is ok if the YT doesn't call the tranche's methods in its constructor
        address computedAddr = trancheFor(adapter, maturity);
        YieldToken yt = new YieldToken(
            computedAddr,
            IBaseAdapter(adapter).underlying(),
            IBaseAdapter(adapter).target(),
            maturity
        );

        // store temporary variables
        _tempArgs.adapter = adapter;
        _tempArgs.maturity = maturity.toUint32();
        _tempArgs.tilt = tilt.toUint16();
        _tempArgs.issuanceFee = issuanceFee.toUint16();
        _tempArgs.yt = address(yt);

        // deploy PT with CREATE2 using adapter and maturity as salt
        // receive callback to initialize the tranche
@>      Tranche tranche = Create2TrancheLib.deploy(adapter, maturity);
@>      if (computedAddr != address(tranche)) revert TrancheAddressMismatch();

        // set back to zero-value for refund
        delete _tempArgs;

        // store the series
        tranches[adapter][maturity] = address(tranche);
        emit TrancheDeployed(maturity, address(tranche), address(yt));

        return address(tranche);
    }

But we are not checking whether the code is correctly deployed on that address or not.

Previously raised similar issue on sherlock link.

Impact

  1. DOS as the contract is not deployed and hence User will not be able to interact with it.
  2. Noone can create a new tranch with the same adapter and maturity value as code is not reverting and are storing the value here and checking it here.

Code Snippet

Code

Tool used

Manual Review

Recommendation

The recommended mitigation is to check whether the code is deployed correctly to not by adding require(extcodesize(address(tranche)) != 0 ).