The function setBooster() within FlywheelCore.sol is unreachable from inside the owner contract under the current layout.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/rewards/base/FlywheelCore.sol#L137-L141 https://github.com/code-423n4/2023-05-maia/blob/main/src/gauges/factories/BribesFactory.sol#L82-L87 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/factories/TalosStrategyStakedFactory.sol#L47-L52

Vulnerability details

vulnerability explained:

This issue actually occurs twice throughout the ecosystem. FlywheelCore is inherited by both FlywheelCoreInstant and FlywheelCoreStrategy, and these two contracts are initialized within BribesFactory(creates FlywheelCoreStrategy) and TalosStrategyStakedFactory(creates FlywheelCoreInstant). When these contracts are created, the two factory contracts automatically set themselves as the owner. Neither one of them has a function that calls setBooster() (which has the onlyOwner modifier). This renders the function unusable.

Impact

This can have a moderate impact on the protocol. Within the docs for FlywheelBoosterGaugeWeight, the team comments that the booster can be used for “virtually boosting or otherwise transforming user balances”, as well as for “referrals, vote-escrow, or other strategies”.
However, none of this logic appears to be present in the current implementation of FlywheelBoosterGaugeWeight (which is set in the constructor of both FlywheelCoreInstant or FlywheelCoreStrategy). This strongly implies that new booster contracts will need to be added in the future to the two flywheel contracts as new user perks become available. However, there is currently no way to add new booster contracts to FlywheelCoreInstant or FlywheelCoreStrategy because setBooster() is unreachable, as described above.

Overall, having a booster contract is optional and not crucial to the fundamental logic of flyWheelCore. Nonetheless, FlywheelBoosterGaugeWeight is useful for facilitating user perks and benefits, and it’s necessary that updated versions of this contract can be added or swapped later on.

Proof of Concept

The following link is from inside BribesFactory.sol, and is where FlywheelCoreStrategy is created. One of the inputs is address(this), and this is where the BribesFactory automatically sets itself as the owner.

https://github.com/code-423n4/2023-05-maia/blob/main/src/gauges/factories/BribesFactory.sol#L82-L87

FlywheelCore flywheel = new FlywheelCore(
        bribeToken,
        FlywheelBribeRewards(address(0)),
        flywheelGaugeWeightBooster,
        address(this)
    );

The next link is from inside TalosStrategyStakedFactory.sol, and is where FlywheelCoreInstant is created. Just like before, address(this) is where the TalosStrategyStakedFactory automatically sets itself as the owner. Neither BribesFactory or TalosStrategyStakedFactory has functionality that allows it to call setBooster() (which is shown below).

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/factories/TalosStrategyStakedFactory.sol#L47-L52

flywheel = new FlywheelCoreInstant(
        address(_boostAggregatorFactory.hermes()),
        IFlywheelRewards(address(0)),
        IFlywheelBooster(address(0)),
        address(this)
    );

The function setBooster() inside of of FLywheelCore.sol.

https://github.com/code-423n4/2023-05-maia/blob/main/src/rewards/base/FlywheelCore.sol#L137-L141

function setBooster(IFlywheelBooster newBooster) external onlyOwner {
    flywheelBooster = newBooster;

    emit FlywheelBoosterUpdate(address(newBooster));
}

Tools Used

The Maia team may have missed this because in both the testing contract (FlywheelInstantTest.t.sol and FlywheelStrategyTest.t.sol), the owner is not set to the factory contracts, but to the testing contracts themselves. This means that setBooster was updated successfully within the testing contracts, but it would not be updated successfully in real life through the factory contracts (who are really the owner).

https://github.com/code-423n4/2023-05-maia/blob/main/test/rewards/FlywheelInstantTest.t.sol#L33-L38

flywheel = new FlywheelCoreInstant(
        address(rewardToken),
        MockRewardsInstant(address(0)),
        IFlywheelBooster(address(0)),
        address(this)
    );

https://github.com/code-423n4/2023-05-maia/blob/main/test/rewards/FlywheelInstantTest.t.sol#L73-L76

function testSetFlywheelBooster(IFlywheelBooster _booster) public {
    flywheel.setBooster(_booster);
    assertEq(address(flywheel.flywheelBooster()), address(_booster));
}

Recommended Mitigation Steps

Fortunately, there is an easy solution. A restricted function can be added within both BribesFactory and TalosStrategyStakedFactory that allows the owner of these contracts to call setBooster() within FlywheelCoreInstant and FlywheelCoreStrategy. This way, new booster contracts can be swapped or added in.

function updateBooster(address bribeToken, address _newflywheelGaugeWeightBooster) external 
onlyOwner {
flywheelTokens[bribeToken].setBooster(_newflywheelGaugeWeightBooster);
}

Assessed type

Access Control

[c4-judge]

trust1995 marked the issue as duplicate of #340

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 changed the severity to QA (Quality Assurance)