Trader Joe contest - harleythedog's results

Handle

harleythedog

Vulnerability details

Impact

In LaunchEvent.sol, the function _safeTransferAVAX is as follows:

function _safeTransferAVAX(address _to, uint256 _value) internal {
    (bool success, ) = _to.call{value: _value}(new bytes(0));
    require(success, "LaunchEvent: avax transfer failed");
}

This function is utilized in a few different places in the contract. According to the Solidity docs), "The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed".

As a result, it is possible that this call will fail, but _safeTransferAVAX will not notice anything went wrong. In particular, it is possible that the address rocketJoeFactory.penaltyCollector() is a deleted contract (perhaps a security flaw was found and selfdestruct was called so that users know to use an updated smart contract), but _safeTransferAVAX will not revert. If rocketJoeFactory.penaltyCollector() is indeed a non-existent contract, it would be better for _safeTransferAVAX to revert until an admin can manually correct the penaltyCollector in the factory.

For reference, see a similar high severity reported in a Uniswap audit here (report # 9): https://github.com/Uniswap/v3-core/blob/main/audits/tob/audit.pdf

Proof of Concept

See _safeTransferAVAX here. See how this function is called with _to as rocketJoeFactory.penaltyCollector() here, but this contract's existence is not verified, which is a problem as described above.

Tools Used

Inspection.

Recommended Mitigation Steps

Check for contract existence on low-level calls, so that failures are not missed.

Handle

harleythedog

Vulnerability details

Impact

The whole point of the LaunchEvent.sol contract is to gather token and WAVAX to be added to a JoePair. Looking at the JoeRouter02.sol code here, I am not seeing anything preventing a griefer from creating a pair for token and WAVAX before the LaunchEvent.sol function createPair can be called. If a griefer does this, and adds a tiny bit of liquidity, there is essentially no cost to the griefer, and all calls to createPair will revert due to the require statement here.

This jeopardizes the whole point of the contract, and the griefing attack would be easy to time (since LaunchEvent.sol has to complete 3 stages before createPair can even be called), so I consider this a high severity issue.

Proof of Concept

See the code for createPair here.

Tools Used

Inspection.

Recommended Mitigation Steps

The require statements in createPair should be reconsidered. Requiring that there be zero supply if the pair already exists is too restrictive. Perhaps only a certain amount of liquidity should be allowed, but certainly more than zero should be allowed, or else anyone can easily grief as described above.