ExchangeCommitFacet Manual Review Findings

ECF-01M: Incorrect Message Sender Emission

Type	Severity	Location
Logical Fault		ExchangeCommitFacet.sol:L916

Description:

The ExchangeCommitFacet::handleDRFeeCollection function will incorrectly emit the msg.sender as the executor of the fee request whereas other event emissions will use the properly-evaluated ProtocolBase::_msgSender.

Impact:

The DRFeeRequested event will be emitted with a misleading executor data point in the current implementation.

Example:

Copy
910// Emit event for DR fee request
911emit IBosonFundsBaseEvents.DRFeeRequested(
912    _exchangeId,
913    exchangeToken,
914    _drFeeAmount,
915    _disputeTerms.mutualizerAddress,
916    msg.sender
917);

Recommendation:

We advise the proper message sender to be resolved to ensure consistency across the codebase.

Alleviation (efd5d1a8f23c3bca7c25273ea4c912a367250119):

The event emission was updated to properly utilize the ProtocolBase::_msgSender, addressing this exhibit.

ECF-02M: Inexistent Validation of Mutualizer Interface Conformity

Type	Severity	Location
Input Sanitization		ExchangeCommitFacet.sol:L962

Description:

In contrast to the OfferBase::storeOffer function, the ExchangeCommitFacet::addSellerParametersToBuyerOffer function does not validate that the _sellerParams.mutualizerAddress adheres to the expected mutualizer interface.

Impact:

A non-functional mutualizer can be presently configured in the ExchangeCommitFacet::addSellerParametersToBuyerOffer function.

Example:

Copy
934function addSellerParametersToBuyerOffer(
935    address _committer,
936    uint256 _offerId,
937    SellerOfferParams calldata _sellerParams
938) internal returns (BosonTypes.Offer storage offer) {
939    offer = getValidOffer(_offerId);
940    if (offer.priceType != PriceType.Static) revert InvalidPriceType();
941    if (offer.creator != OfferCreator.Buyer) revert InvalidOfferCreator();
942
943    (bool sellerExists, uint256 sellerId) = getSellerIdByAssistant(_committer);
944    if (!sellerExists) revert NotAssistant();
945    offer.sellerId = sellerId;
946
947    {
948        ProtocolLib.ProtocolLookups storage lookups = protocolLookups();
949        if (_sellerParams.collectionIndex > 0) {
950            if (lookups.additionalCollections[sellerId].length < _sellerParams.collectionIndex) {
951                revert NoSuchCollection();
952            }
953            offer.collectionIndex = _sellerParams.collectionIndex;
954        }
955
956        validateRoyaltyInfo(lookups, protocolLimits(), sellerId, _sellerParams.royaltyInfo);
957
958        offer.royaltyInfo[0] = _sellerParams.royaltyInfo;
959
960        if (_sellerParams.mutualizerAddress != address(0)) {
961            DisputeResolutionTerms storage disputeTerms = fetchDisputeResolutionTerms(_offerId);
962            disputeTerms.mutualizerAddress = _sellerParams.mutualizerAddress;
963        }
964
965        emit BuyerInitiatedOfferSetSellerParams(_offerId, sellerId, _sellerParams, _committer);
966    }
967}

Recommendation:

We advise such validation to be imposed, ensuring the updated mutualizer satisfies the expected interface.

Alleviation (efd5d1a8f23c3bca7c25273ea4c912a367250119):

The mutualizer interface validation has been introduced to the ExchangeCommitFacet::addSellerParametersToBuyerOffer function as advised.

ECF-03M: Fee Mutualizer Fund Extraction via Re-Entrancy

Type	Severity	Location
Logical Fault		ExchangeCommitFacet.sol:L472, L554, L637, L664, L890, L901

Description:

The BPIP-8 standard introduced to the Boson Protocol system involves a novel dispute resolution fee mutualizer implementation meant to facilitate the provision of dispute resolution fees at a premium instead of forcing buyers or sellers to contribute them.

A complex re-entrancy attack has been introduced as a result of this feature due to how the dispute resolution fee is requested and captured.

Specifically, the ExchangeCommitFacet::handleDRFeeCollection function will measure the balance before and after the IDRFeeMutualizer::requestDRFee function call. A re-entrancy would permit the same balance delta (i.e. single transfer) to be measured as if multiple transfers occurred, thereby crediting an arbitrary number n of dispute resolution fees through a single transfer.

While most of the codebase is protected against re-entrancy attacks, a previously-established security assumption is now breached in the ExchangeCommitFacet::onPremintedVoucherTransferred function.

As the function is not protected against re-entrancy guards, it is possible to execute the described re-entrancy attack through the function via single user controlled buyers and sellers.

As multiple dispute resolution fees would be credited through a single transfer, fund extraction is possible by simply refunding all credited dispute resolution fees through the FundsBase::releaseFunds path.

Impact:

A re-entrancy edge case has been introduced in the Boson Protocol system through the BPIP-8 implementation that can result in fund loss for the protocol.

Example:

Copy
889// Use mutualizer: request fee
890uint256 balanceBefore = getBalance(exchangeToken);
891
892// Request DR fee from mutualizer
893bool success = IDRFeeMutualizer(mutualizer).requestDRFee(
894    _offer.sellerId,
895    _drFeeAmount,
896    exchangeToken,
897    _exchangeId,
898    _disputeTerms.disputeResolverId
899);
900
901uint256 balanceAfter = getBalance(exchangeToken);

Recommendation:

There are multiple approaches in addressing this particular issue, with one being the validation of mutualizer addresses against a list of authorized implementations.

An alternative approach would be to introduce an "atomic" re-entrancy guard to the ExchangeCommitFacet::handleDRFeeCollection function block that prevents this particular function from being re-entered.

Alleviation (efd5d1a8f23c3bca7c25273ea4c912a367250119):

A localized re-entrancy guard was introduced to the ExchangeCommitFacet::onPremintedVoucherTransferred function preventing re-entrancies from triggering automatic commitments to offers more than once in a single transfer, thereby eliminating the re-entrancy risk outlined and thus alleviating this exhibit.