code423n4 commented 2 years ago

Handle

Czar102

Vulnerability details

Impact

A bad actor can call the initialize() function of RocketJoeToken right after deployment, so that it will be useless to the deployer. A deployer might have already breadcasted a transaction deploying RocketJoeFactory and it will fail as the constructor requires RocketJoeToken's initialize() call not to revert.

Proof of Concept

    function initialize() external {
        require(
            address(rocketJoeFactory) == address(0),
            "RocketJoeToken: already initialized"
        );

        rocketJoeFactory = IRocketJoeFactory(msg.sender);
    }

Tools Used

Manual analysis

Recommended Mitigation Steps

Make owner set the rocketJoeFactory variable once, after RocketJoeFactory is deployed.

cryptofish7 commented 2 years ago

Duplicate of #8

dmvt commented 2 years ago

Assets are not at risk. The worst case is that there is lost gas and the contracts need to be redeployed. Consider creating these contracts and calling their initialize functions in a factory to mitigate the issue if not adding the guard.

1 — Low (L): vulns that have a risk of 1 are considered “Low” severity when assets are not at risk. Includes state handling, function incorrect as to spec, and issues with comments.

code-423n4 / 2022-01-trader-joe-findings