PendlePowerFarmTokenFactory::deploy() does not revert if contract deployment failed

[hats-bug-reporter[bot]]

Github username: @hunter-w3b Twitter username: hunter_w3b Submission hash (on-chain): 0x80f4ed45b03501f4d7917571e7e6d05b91795e08fc1849dcbb85a668bb85ddb9 Severity: medium

Description: Description\ In PendlePowerFarmTokenFactory.sol, the deploy() function, which is used to deploy contracts with the CREATE2 opcode, is as shown:

function deploy(
        address _underlyingPendleMarket,
        string memory _tokenName,
        string memory _symbolName,
        uint16 _maxCardinality
    )
        external
        returns (address)
    {
        if (msg.sender != PENDLE_POWER_FARM_CONTROLLER) {
            revert DeployForbidden();
        }

        return _clone(
            _underlyingPendleMarket,
            _tokenName,
            _symbolName,
            _maxCardinality
        );
    }

    function _clone(
        address _underlyingPendleMarket,
        string memory _tokenName,
        string memory _symbolName,
        uint16 _maxCardinality
    )
        private
        returns (address pendlePowerFarmTokenAddress)
    {
        bytes32 salt = keccak256(
            abi.encodePacked(
                _underlyingPendleMarket
            )
        );

        bytes20 targetBytes = bytes20(
            IMPLEMENTATION_TARGET
        );

        assembly {

            let clone := mload(0x40)

            mstore(
                clone,
                0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000
            )

            mstore(
                add(clone, 0x14),
                targetBytes
            )

            mstore(
                add(clone, 0x28),
                0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000
            )

            pendlePowerFarmTokenAddress := create2( // @audit create2 does not check when revert 
                0,
                clone,
                0x37,
                salt
            )
        }

        PendlePowerFarmToken(pendlePowerFarmTokenAddress).initialize(
            _underlyingPendleMarket,
            PENDLE_POWER_FARM_CONTROLLER,
            _tokenName,
            _symbolName,
            _maxCardinality
        );
    }

The create2 opcode returns address(0) if contract deployment reverted. However, as seen from above, deploy() does not check if the deployment address is address(0).

This is an issue as PendlePowerFarmController.sol::addPendleMarket() will not revert when deployment of the addPendleMarket contract fails:

        address pendleChild = PENDLE_POWER_FARM_TOKEN_FACTORY.deploy(
            _pendleMarket,
            _tokenName,
            _symbolName,
            _maxCardinality
        );

Therefore, if the origination fee is enabled for the protocol, users that call addPendleMarket() will pay the origination fee even if the market was not deployed.

Additionally, the _pendleMarket address will be registered in the PendlePowerFarmController contract and added to pendleChildAddress. This will cause both sets to become inaccurate if deployment failed as market would be an address that has no code.

This also leads to more problems if a user attempts to call addPendleMarket() with the same _pendleMarket, _tokenName and _symbolName.

Since the market address has already been registered, addPendleMarket() will revert when called for a second time:

        if (pendleChildAddress[_pendleMarket] > ZERO_ADDRESS) {
            revert AlreadySet();
        }

As such, if a user calls addPendleMarket() and market deployment fails, they cannot call addPendleMarket() with the same set of parameters ever again.

Recommended Mitigation\ In _clone(), consider checking if the deployment address is address(0), and reverting if so:

            pendlePowerFarmTokenAddress := create2( // @audit create2 does not check when revert 
                0,
                clone,
                0x37,
                salt
            )
+     if iszero(pendlePowerFarmTokenAddress) {
+         mstore(0x00, 0x30116425) // DeploymentFailed()
+         revert(0x1c, 0x04)
+     }

[vm06007]

this is already mitigated and taken care when initialize() is called one instruction below.

if pendlePowerFarmTokenAddress results in 0x0 this will revert.

PendlePowerFarmToken(pendlePowerFarmTokenAddress).initialize(
    _underlyingPendleMarket,
    PENDLE_POWER_FARM_CONTROLLER,
    _tokenName,
    _symbolName,
    _maxCardinality
);

you can check this this test scenario:

// SPDX-License-Identifier: -- WISE ---

pragma solidity =0.8.24;

interface IChild {
    function initialize()
        external;
}

contract TargetWithInitialize {

    function initialize() 
        external
    {
        // maybe emit event here
    }

}

contract CallFunctionOnZeroAddress {

    function doEmptyCall(
        IChild _target
    )
        external 
    {
        _target.initialize();
    }
}

1) deploy CallFunctionOnZeroAddress() 2) call doEmptyCall(0x0000000000000000000000000000000000000000) 3) see transaction reverts

1) deploy TargetWithInitialize() and copy address 2) call doEmptyCall() with address from step 1 3) see transaction passes

[vm06007]

@hunter_w3b, let me know if you concur with displayed examples above. @vonMangoldt or @Foon256 can add more details if needed