ConfigManager Manual Review Findings

CMR-01M: Potentially Misleading Events

Type	Severity	Location
Logical Fault		ConfigManager.sol: • I-1: L117-L118 • I-2: L125-L126 • I-3: L135-L136 • I-4: L143-L144 • I-5: L186-L188 • I-6: L192-L196 • I-7: L203-L205 • I-8: L213-L217 • I-9: L223-L229 • I-10: L234-L240

Description:

The referenced statements will result in an event being emitted regardless of whether they were no-op actions, as repeating the same action (i.e. removal or addition) for the same item is permitted.

Example:

Copy
122function delistFuses(address[] calldata fuses) external onlyRole(DEFAULT_ADMIN_ROLE) {
123    if (fuses.length == 0) revert EmptyArray();
124    for (uint256 i; i < fuses.length; i++) {
125        _whitelistedFuses.remove(fuses[i]);
126        emit FuseDelisted(fuses[i]);
127    }
128}

Recommendation:

We advise the code to emit the relevant events solely when an action has been performed by checking the return value of each EnumerableSet::add and EnumerableSet::remove operation, with true indicating a successful operation.

Alleviation (fdf0694b5c38834fa5a20cb2765766924d34f5d4):

The code was updated across all instances to emit events solely when actual mutations are performed on the storage entries of the contract, addressing this exhibit properly.

CMR-02M: Inexistent Market ID Length Checks

Type	Severity	Location
Input Sanitization		ConfigManager.sol: • I-1: L317-L328 • I-2: L386-L392

Description:

The referenced statements do not ensure that the marketIds array has a non-zero length, permitting validation to be bypassed for empty market ID arrays in contrast to all other functions of the ConfigManager implementation.

Impact:

An OPERATOR_ROLE is able to issue an empty marketIds but non-zero dependencies array to an IPlasmaVaultGovernance::updateDependencyBalanceGraphs call without any validation as the marketIds array would be empty.

Example:

Copy
317function pushDependencyBalanceGraphs(
318    address vault,
319    uint256[] calldata marketIds,
320    uint256[][] calldata dependencies
321) external onlyRole(OPERATOR_ROLE) {
322    if (vault == address(0)) revert ZeroAddress();
323    for (uint256 i; i < marketIds.length; i++) {
324        if (!_whitelistedMarketIds.contains(marketIds[i])) revert MarketIdNotWhitelisted(marketIds[i]);
325    }
326    IPlasmaVaultGovernance(vault).updateDependencyBalanceGraphs(marketIds, dependencies);
327    emit DependencyBalanceGraphsPushed(vault, marketIds);
328}

Recommendation:

We advise these code segments to validate the length of the marketIds array, ensuring consistency in the level of sanitization imposed across the contract.

Alleviation (fdf0694b5c):

This exhibit has been partially addressed as only the first of the two instances has been alleviated.

Alleviation (c39cbc38aa):

The Tesseract team provided us with a follow-up commit hash addressing the second leg of the final remaining exhibit.

CMR-03M: Inexistent Validation of Whitelist State

Type	Severity	Location
Logical Fault		ConfigManager.sol: • I-1: L255-L260 • I-2: L273-L279 • I-3: L301-L305

Description:

While pushing addition updates to the vaults is accompanied by a whitelist check, removals do not perform any validation.

As such, the authority of the OPERATOR_ROLE inadvertently supersedes the authority of the DEFAULT_ADMIN_ROLE when removing fuses, reward fuses, and balance fuses.

Impact:

A compromised OPERATOR_ROLE can significantly harm the system by bypassing the DEFAULT_ADMIN_ROLE authority and removing whichever fuses it wishes from integrated vaults.

Example:

Copy
245function pushAddFuses(address vault, address[] calldata fuses) external onlyRole(OPERATOR_ROLE) {
246    if (vault == address(0)) revert ZeroAddress();
247    if (fuses.length == 0) revert EmptyArray();
248    for (uint256 i; i < fuses.length; i++) {
249        if (!_whitelistedFuses.contains(fuses[i])) revert FuseNotWhitelisted(fuses[i]);
250    }
251    IPlasmaVaultGovernance(vault).addFuses(fuses);
252    emit FusesPushed(vault, fuses);
253}
254
255function pushRemoveFuses(address vault, address[] calldata fuses) external onlyRole(OPERATOR_ROLE) {
256    if (vault == address(0)) revert ZeroAddress();
257    if (fuses.length == 0) revert EmptyArray();
258    IPlasmaVaultGovernance(vault).removeFuses(fuses);
259    emit FusesRemoved(vault, fuses);
260}

Recommendation:

We advise the code to ensure that the elements being removed are not located within the whitelist to ensure only removed elements can have their removal pushed to the respective vault implementations that rely on them.

Alleviation (fdf0694b5c38834fa5a20cb2765766924d34f5d4):

The Tesseract team evaluated this exhibit and opted to acknowledge it as by-design based on the fact that they consider the DEFAULT_ADMIN_ROLE to configure what can be added to a vault and the OPERATOR_ROLE to actually be responsible for what is added and removed from a vault.

As such, we consider this exhibit properly invalidated by design.