ExchangeHandlerFacet Manual Review Findings

EHF-01M: Breaking Change of Single-Point-of-Entry

Type	Severity	Location
Logical Fault		ExchangeHandlerFacet.sol:L76, L127

Description:

The ExchangeHandlerFacet::commitToOffer function was the single point of entry of the particular facet for committing to offers within the Boson Protocol system.

A new feature around enhanced token gating split the functionality of committing to offers to two implementations; ExchangeHandlerFacet::commitToOffer and ExchangeHandlerFacet::commitToConditionalOffer. The issue with the original implementation is that it will currently revert if the commit cannot occur due to the offer being part of a group with a condition.

Impact:

The severity of this exhibit depends on the current adoption of the Boson Protocol system and whether the Boson Protocol team has reached out to these parties to verify that the upgrade is compatible with their systems.

Example:

Copy
76function commitToOffer(
77    address payable _buyer,
78    uint256 _offerId
79) external payable override exchangesNotPaused buyersNotPaused nonReentrant {
80    // Make sure buyer address is not zero address
81    require(_buyer != address(0), INVALID_ADDRESS);
82
83    Offer storage offer = getValidOffer(_offerId);
84
85    // For there to be a condition, there must be a group.
86    (bool exists, uint256 groupId) = getGroupIdByOffer(offer.id);
87    if (exists) {
88        // Get the condition
89        Condition storage condition = fetchCondition(groupId);
90
91        // Make sure group doesn't have a condition. If it does, use commitToConditionalOffer instead.
92        require(condition.method == EvaluationMethod.None, GROUP_HAS_CONDITION);
93    }
94
95    commitToOfferInternal(_buyer, offer, 0, false);
96}

Recommendation:

As the Boson Protocol system has most likely been integrated by external parties, these external parties will no longer be able to interact with conditional offers via the ExchangeHandlerFacet::commitToOffer function that may lead to significant consequences depending on whether those parties possess upgradeable code or not.

We advise the ExchangeHandlerFacet::commitToOffer function to redirect its call to ExchangeHandlerFacet::commitToConditionalOffer with a _tokenId of 0, permitting a variety of conditions to still be satisfied.

In general, the new per-token-ID-commit token gating feature should be backwards-compatible with the original restriction system and assume default values for the _tokenId if it is unspecified.

Alleviation (2b9f60b6c3):

The Boson Protocol team stated that they do not wish to expose the same functionality across two different functions and that they consider the change safe to apply to the system in its current state.

We would like to state that the Boson Protocol team has not confirmed that integrators of their systems will adequately react to this non-backwards-compatible change and we advise the Boson Protocol team to proactively reach out to potentially affected parties rather than dealing with them after-the-fact.

Alleviation (584e7d054c):

The Boson Protocol team re-evaluated this exhibit and stated that the sole integrators of their system at its current state are internal and have been prepared for this change.

As such, we consider this exhibit nullified.

EHF-02M: Restrictive Migration Mechanism

Type	Severity	Location
Language Specific		ExchangeHandlerFacet.sol:L38

Description:

The ExchangeHandlerFacet introduces an upgrade to the BosonVoucher identification system, however, the way the ExchangeHandlerFacet differentiates between the two is based on a "snapshot" EXCHANGE_ID_2_2_0 beyond which IDs are assumed to be of the new representation format.

Impact:

This exhibit acts as a warning of a potential caveat of the EXCHANGE_ID_2_2_0 system that the Boson Protocol may not be aware of as well as a way to make the distinction between legacy and new-style IDs more robust.

Example:

Copy
37constructor(uint256 _firstExchangeId2_2_0) {
38    EXCHANGE_ID_2_2_0 = _firstExchangeId2_2_0;
39}

Recommendation:

While this approach is correct, it will misbehave if the ExchangeHandlerFacet and BosonVoucher implementations are not upgraded properly.

Namely, the exchange and buyer regions of the protocol will have to be paused before the BosonVoucher implementations are upgraded as otherwise vouchers may be issued between a protocol update and its execution, causing the minted IDs to be improperly parsed.

Alternatively, the code could differentiate between the new and legacy type IDs by evaluating the upper 128 bits of the ID. If the upper 128 bits are zero, then it is of the legacy type as a zero-value offer ID is not possible.

Alleviation (2b9f60b6c3323fd234b570089ceff924cdb5851c):

The Boson Protocol team stated that they will apply proper upgrade procedures to ensure that the ID system does not become corrupt in-between upgrades.

Furthermore, they stated that a legacy ID evaluation system was experimented but was deemed to complex and gas-expensive for a production environment.

As such, we consider this exhibit alleviated based on the fact that the Boson Protocol team will follow proper upgrade procedures for their contracts.

EHF-03M: Bypass of Token Specific Conditions

Type	Severity	Location
Logical Fault		ExchangeHandlerFacet.sol:L148-L154, L156-L158

Description:

As par of the condition overhaul supporting range-based token gating with limited commits per unique token ID, the ExchangeHandlerFacet::authorizeCommit function takes an extra _tokenId argument that is not validated whereas previously it was utilizing the _condition.tokenId.

While the ExchangeHandlerFacet::commitToPreMintedOffer function will automatically utilize the correct tokenId value based on the condition, the ExchangeHandlerFacet::commitToConditionalOffer does not perform this check. As such, it is possible for a group that is meant to be satisfied by a specific NFT ID to be satisfied by any.

Impact:

A Condition that is MultiToken and has a Threshold or that is a SpecificToken condition will not evaluate properly in the ExchangeHandlerFacet::commitToConditionalOffer function in the current iteration of the codebase and can be trivially bypassed.

Example:

Copy
127function commitToConditionalOffer(
128    address payable _buyer,
129    uint256 _offerId,
130    uint256 _tokenId
131) external payable override exchangesNotPaused buyersNotPaused nonReentrant {
132    // Make sure buyer address is not zero address
133    require(_buyer != address(0), INVALID_ADDRESS);
134
135    Offer storage offer = getValidOffer(_offerId);
136
137    // For there to be a condition, there must be a group.
138    (bool exists, uint256 groupId) = getGroupIdByOffer(offer.id);
139
140    // Make sure the group exists
141    require(exists, NO_SUCH_GROUP);
142
143    // Get the condition
144    Condition storage condition = fetchCondition(groupId);
145
146    require(condition.method != EvaluationMethod.None, GROUP_HAS_NO_CONDITION);
147
148    if (condition.length >= 1) {
149        // If condition has length >= 1, check that the token id is in range
150        require(
151            _tokenId >= condition.tokenId && _tokenId < condition.tokenId + condition.length,
152            TOKEN_ID_NOT_IN_CONDITION_RANGE
153        );
154    }
155
156    if (condition.method == EvaluationMethod.Threshold && condition.tokenType != TokenType.MultiToken) {
157        require(_tokenId == 0, INVALID_TOKEN_ID);
158    }
159
160    bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId);

Recommendation:

We advise the code of ExchangeHandlerFacet::commitToConditionalOffer function to be updated, either ensuring that the input _tokenId has been correctly specified or overwriting it with the correct value based on the condition of the group being evaluated.

Alleviation (2b9f60b6c3323fd234b570089ceff924cdb5851c):

The code of ExchangeHandlerFacet::commitToConditionalOffer was updated to properly evaluate whether a SpecificToken evaluation method or a token type of MultiToken has been specified and to validate the provided _tokenId in such a case.

As such, we consider this exhibit fully alleviated.