ProtocolBase Manual Review Findings

PBS-01M: Potentially Unsafe Existence Check

Type	Severity	Location
Language Specific		ProtocolBase.sol:L720

Description:

The linked existence check utilizes a fixed enum value instead of evaluating the actual numeric representation of the enum being equal to 0.

Example:

Copy
704/**
705 * @notice Fetches a condition from storage by exchange id
706 *
707 * @param _exchangeId - the id of the exchange
708 * @return exists - whether one condition exists for the exchange
709 * @return condition - the condition. See {BosonTypes.Condition}
710 */
711function fetchConditionByExchange(uint256 _exchangeId)
712    internal
713    view
714    returns (bool exists, Condition storage condition)
715{
716    // Get the condition slot
717    condition = protocolLookups().exchangeCondition[_exchangeId];
718
719    // Determine existence
720    exists = (_exchangeId > 0 && condition.method != EvaluationMethod.None);
721}

Recommendation:

We advise the actual numeric representation to be utilized that correctly depicts its "unset" status as should the enum declaration change in order the EvaluationMethod.None could be depicting a non-zero state thus invalidating the existence check.

Alleviation (44009967e4f68092941d841e9e0f5dd2bb31bf0b):

The Boson Protocol team stated that they will retain the current code in place and will simply ensure via test suites that the None evaluation method always exist in the first enum slot as that is its intended behaviour. We consider this exhibit as addressed based on the assumption that the Boson Protocol team will guarantee the None value's validity in all iterations of the codebase.