c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/LibStoredInitCode.sol#L76

Vulnerability details

Impact

Permanent DoS of creating a market if anything fails in the market constructor.

Within a create2 call, instead of failing when a revert occurs in the constructor being run, it just returns address(0).

If this occurs within a market that's being created the transaction will not revert in the controller and instead will register the address as a valid market, despite nothing having been created.

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatMarketController.sol#L356

Once this is registered, it will block any future attempts at registration, so any more attempts at creating the market will fail despite it never having been created.

https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/WildcatArchController.sol#L193

Proof of Concept

contract FactoryAssembly { event Deployed(address addr, uint salt);

// 1. Get bytecode of contract to be deployed
// NOTE: _owner and _foo are arguments of the TestContract's constructor
function getBytecode(address _owner, uint _foo) public pure returns (bytes memory) {
    bytes memory bytecode = type(TestContract).creationCode;

    return abi.encodePacked(bytecode, abi.encode(_owner, _foo));
}

// 2. Compute the address of the contract to be deployed
// NOTE: _salt is a random number used to create an address
function getAddress(
    bytes memory bytecode,
    uint _salt
) public view returns (address) {
    bytes32 hash = keccak256(
        abi.encodePacked(bytes1(0xff), address(this), _salt, keccak256(bytecode))
    );

    // NOTE: cast last 20 bytes of hash to address
    return address(uint160(uint(hash)));
}

// 3. Deploy the contract
// NOTE:
// Check the event log Deployed which contains the address of the deployed TestContract.
// The address in the log should equal the address computed from above.
function deploy(bytes memory bytecode, uint _salt) public payable {
    address addr;

    /*
    NOTE: How to call create2

    create2(v, p, n, s)
    create new contract with code at memory p to p + n
    and send v wei
    and return the new address
    where new address = first 20 bytes of keccak256(0xff + address(this) + s + keccak256(mem[p…(p+n)))
          s = big-endian 256-bit value
    */
    assembly {
        addr := create2(
            callvalue(), // wei sent with current call
            // Actual code starts after skipping the first 32 bytes
            add(bytecode, 0x20),
            mload(bytecode), // Load the size of code contained in the first 32 bytes
            _salt // Salt from function arguments
        )

        if iszero(extcodesize(addr)) {
            revert(0, 0)
        }
    }

    emit Deployed(addr, _salt);
}

}

contract TestContract { address public owner; uint public foo;

constructor(address _owner, uint _foo) payable {
    owner = _owner;
    foo = _foo;
    revert();
}

function getBalance() public view returns (uint) {
    return address(this).balance;
}

}

TestContract in this PoC should revert the transaction when being created, but instead the transaction succeeds without TestContract having been created.

Tools Used

Manual

Recommended Mitigation Steps

Add the same check that the normal create has: https://github.com/code-423n4/2023-10-wildcat/blob/c5df665f0bc2ca5df6f06938d66494b11e7bdada/src/libraries/LibStoredInitCode.sol#L35

Assessed type

DoS

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #28

c4-judge commented 1 year ago

MarioPoneder changed the severity to 2 (Med Risk)