ConfigHandlerFacet Manual Review Findings

CHF-01M: Potentially Overly Centralized Configuration Control

Type	Severity	Location
Centralization Concern		ConfigHandlerFacet.sol:L76-L79, L95-L98, L114-L117, L133-L136

Description:

The referenced functions permit the ADMIN role holder of the system to arbitrarily set very sensitive contract components (such as the token address of the protocol).

Example:

Copy
69/**
70 * @notice Sets the address of the Boson Protocol token contract.
71 *
72 * Emits a TokenAddressChanged event.
73 *
74 * @param _tokenAddress - the address of the token contract
75 */
76function setTokenAddress(address payable _tokenAddress) public override onlyRole(ADMIN) nonReentrant {
77    protocolAddresses().token = _tokenAddress;
78    emit TokenAddressChanged(_tokenAddress, msgSender());
79}

Recommendation:

We advise this to be revised and very sensitive configurational variables to only be settable once during the contract's lifetime to avoid centralization risks.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

The Boson Protocol team has opted to retain the current centralization structure in place after evaluating the exhibit given that they wish to apply a progressive decentralization plan to the protocol by utilizing a multi-signature wallet for the initial ADMIN-role activities with a plan to completely abolish control of the account once the protocol has sufficiently matured. As a result, we consider this exhibit as addressed based on the principle that the Boson Protocol team will act in good faith with regards to the protocol when administering ADMIN-role related activities as no adequate decentralized solution currently exists for the protocol.

CHF-02M: Inexistent Validation of Numeric Configuration

Type	Severity	Location
Input Sanitization		ConfigHandlerFacet.sol:L205, L224, L243, L262, L281, L300, L324, L348, L432, L517, L536

Description:

The linked numeric configurational parameters are not properly sanitized which could lead to significant protocol consequences, such as the maxTokensPerWithdrawal value being set to 0 which would in turn not allow tokens to be withdrawn from the system.

Impact:

A zero value for most of these parameters would lead to misbehaviours or inoperable system components such as the illustrated maximum tokens per withdrawal preventing any withdrawal to be performed in the FundsHandlerFacet implementation.

Example:

Copy
274/**
275 * @notice Sets the maximum numbers of tokens that can be withdrawn in a single transaction
276 *
277 * Emits a mMxTokensPerWithdrawalChanged event.
278 *
279 * @param _maxTokensPerWithdrawal - the maximum length of token list when calling {FundsHandlerFacet.withdraw}
280 */
281function setMaxTokensPerWithdrawal(uint16 _maxTokensPerWithdrawal) public override onlyRole(ADMIN) nonReentrant {
282    protocolLimits().maxTokensPerWithdrawal = _maxTokensPerWithdrawal;
283    emit MaxTokensPerWithdrawalChanged(_maxTokensPerWithdrawal, msgSender());
284}

Recommendation:

We advise a non-zero check to be imposed on all referenced input arguments to ensure the contract is configured properly at all times.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

A non-zero check has been introduced across all referenced setter functions ensuring that the contract is properly configured in relation to its numerical values at all times.