PoolDeployer will lose all gas yield due to incorrect claiming implementation

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-02-thruster/blob/3896779349f90a44b46f2646094cb34fffd7f66e/thruster-protocol/thruster-clmm/contracts/ThrusterPoolDeployer.sol#L46

Vulnerability details

Impact

Incorrect gas-claiming implementation will cause PoolDeployer to lose all gas yield.

Proof of Concept

When claiming gas yield on Blast, the claiming contract address is always required as input argument regardless of claiming function invoked (claimMaxGas(), claimAllGas(), etc).

The problem is in one instance, this input argument is hardcoded to address(0), which will disable gas claiming functionality due to incorrect contract address being passed.

Based on Blast doc, this is the correct invocation example:

  // Note: in production, you would likely want to restrict access to this
  function claimMyContractsGas() external {
    BLAST.claimAllGas(address(this), msg.sender);
  }

However, in PoolDeployer.sol. address(0) is used instead of address(this).

//thruster-protocol/thruster-clmm/contracts/ThrusterPoolDeployer.sol
    function claimGas(
        address _recipient
    ) external override onlyFactory returns (uint256 amount) {
         //@audit should be address(this)
|>       amount = IBlast(BLAST).claimMaxGas(address(0), _recipient);
    }

(https://github.com/code-423n4/2024-02-thruster/blob/3896779349f90a44b46f2646094cb34fffd7f66e/thruster-protocol/thruster-clmm/contracts/ThrusterPoolDeployer.sol#L46)

And since claimGas() is the only function that can claim gas yield in ThrusterPoolDeployer.sol, all gas yield earned by ThrusterPoolDeployer.sol will be lost.

Tools Used

Manual

Recommended Mitigation Steps

Use address(this) instead of address(0).

Assessed type

Error

[jooleseth]

This is the same as #24. This should not qualify as a High Risk, because there is no loss of user funds, we will consider this a medium risk. This typo is acknowledged and discovered a few days ago as well after the start of the audit. Initially, Blast documentation had said to use the address(0) before switching to address(this).

[c4-judge]

0xleastwood changed the severity to 2 (Med Risk)

[c4-judge]

0xleastwood marked the issue as duplicate of #24

[c4-judge]

0xleastwood marked the issue as duplicate of #24

[c4-judge]

0xleastwood marked the issue as satisfactory