Tadle

Summary

The initialize() function, which sets crucial system parameters, can be invoked multiple times by the contract owner.

Vulnerability Details

The initialize() function is defined as follows:

function initialize(
uint256 _basePlatformFeeRate,
uint256 _baseReferralRate
) external onlyOwner {
basePlatformFeeRate = _basePlatformFeeRate;
baseReferralRate = _baseReferralRate;
}

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/SystemConfig.sol#L25C4-L32C1

Key issues:

1. No initialization guard: The function lacks a mechanism to prevent multiple calls.

2. Overwritable critical values: basePlatformFeeRate and baseReferralRate can be changed arbitrarily and frequently.

3. Lack of events: Changes to these critical parameters are not logged, reducing transparency.

Impact

Frequent changes to base rates could destabilize the entire economic model of the system. Unpredictable changes in fee structures may lead to loss of user confidence. For instance, a user can call get getBaseReferralRate() and get a certain value. After a while, call getBaseReferralRate() again and get a different value due to sudden change by the owner.
And without proper event logging, it's challenging to track changes to these parameters.

Tools Used

Manual review

Recommendations

Implement an initialization guard:
bool private initialized;

function initialize(uint256 _basePlatformFeeRate, uint256 _baseReferralRate) external onlyOwner {
require(!initialized, "Contract is already initialized");
basePlatformFeeRate = _basePlatformFeeRate;
baseReferralRate = _baseReferralRate;
initialized = true;
emit Initialized(_basePlatformFeeRate, _baseReferralRate);
}

Create separate functions for updating these values if needed:

function updateBasePlatformFeeRate(uint256 _newRate) external onlyOwner {
basePlatformFeeRate = _newRate;
emit BasePlatformFeeRateUpdated(_newRate);
}

function updateBaseReferralRate(uint256 _newRate) external onlyOwner {````baseReferralRate = _newRate;````emit BaseReferralRateUpdated(_newRate);````}