Minato7namikazi - Incorrect Incentive Initialization in BoostCore!

[sherlock-admin4]

Minato7namikazi

High

Incorrect Incentive Initialization in BoostCore!

Summary

https://github.com/sherlock-audit/2024-06-boost-aa-wallet/blob/78930f2ed6570f30e356b5529bd4bcbe5194eb8b/boost-protocol/packages/evm/contracts/BoostCore.sol#L266

The line:

incentives[i] = AIncentive(_makeTarget(type(AIncentive).interfaceId, targets_[i], false));

is missing the case where the incentive might be a pre-existing clone rather than a base implementation that needs to be cloned.

The bug is that it's always treating the incentive as if it needs to be cloned (by passing false as the third argument to _makeTarget), but it should handle both cases:

1. When the incentive is a base implementation that needs to be cloned
2. When the incentive is already a clone that just needs to be used as-is

To fix this, we should check the isBase property of the target. Here's the corrected version:

incentives[i] = AIncentive(_makeTarget(type(AIncentive).interfaceId, targets_[i], targets_[i].isBase));

This change ensures that:

• If targets_[i].isBase is true, it will clone and initialize the incentive.
• If targets_[i].isBase is false, it will use the existing instance without cloning.

This correction aligns with the project's design of allowing both new clones and pre-existing instances to be used as incentives in a Boost.

The bug can be triggered because:

1. In the _makeIncentives function, there's a check that ensures the target is a base implementation:

if (!targets_[i].isBase) {
    revert BoostError.InvalidInstance(type(AIncentive).interfaceId, targets_[i].instance);
}

2. However, the subsequent call to _makeTarget always passes false as the third argument:

incentives[i] = AIncentive(_makeTarget(type(AIncentive).interfaceId, targets_[i], false));

This creates a contradiction: the function requires the target to be a base implementation, but then tells _makeTarget not to clone it.

Impact and PoC flow:

1. A user attempts to create a new Boost with a custom incentive base implementation.
2. They correctly set isBase to true in the Target struct for their incentive.
3. The createBoost function is called, which in turn calls _makeIncentives.
4. _makeIncentives passes the validity check for isBase.
5. However, when _makeTarget is called, it receives false as the third argument.
6. Inside _makeTarget, the _maybeClone function is called:

function _maybeClone(BoostLib.Target memory target_, bool shouldInitialize_) internal returns (address instance) {
    instance = target_.isBase ? target_.instance.clone() : target_.instance;
    if (target_.isBase && shouldInitialize_) {
        ACloneable(instance).initialize(target_.parameters);
    }
}

7. Because shouldInitialize_ is false, the cloned instance is never initialized.
8. The uninitialized clone is then used as the incentive for the Boost.

This leads to a serious issue where the incentive contract is deployed but not properly initialized, potentially causing unexpected behavior or failures when interacting with the Boost later.

For example, if the incentive has critical parameters that should be set during initialization (like reward amounts or limits), these would remain unset, possibly leading to funds being locked or the incentive not functioning as intended.