Description

Note that the admin can configure a fee of more than 100%:

function setFeeBasisPoints(
    uint256 _feeBasisPoints
) public onlyRole(CONFIG_ADMIN_ROLE) returns (uint256) {
    feeBasisPoints = _feeBasisPoints;
    return feeBasisPoints;
}

Among other potential places in the code, the following multiplication in _feeOnRaw could revert:

function _withdraw(
    address caller,
    address receiver,
    address owner,
    uint256 assets,
    uint256 shares
) internal virtual override {
!    uint256 fee = _feeOnRaw(assets, _getFeeBasisPoints()); // in units of underlying asset
    address recipient = _getFeeRecipient();
    // [...]
}

// [...]
function _feeOnRaw(uint256 assets, uint256 _feeBasisPoints) private pure returns (uint256) {
!    return assets.mulDiv(_feeBasisPoints, _BASIS_POINT_SCALE, Math.Rounding.Ceil);
}

Impact

The admin could block redemptions by preventing _withdraw from being able to be executed.

Recommendations

This centralization risk may be of low concern if the admin is a governance contract or multi-sig. However, we recommend limiting the fee to 100% (feeBasisPoints < _BASIS_POINT_SCALE) to be logically consistent and easily reduce centralization risk.

To further limit centralization risk, consider further limiting the fee (e.g., to 5%) so that users do not have to trust the admin as much.

Remediation

This issue has been acknowledged by Solera Markets, and a fix was implemented in PR #12↗.