Omniscia Boson Protocol Audit
ExchangeCommitFacet Manual Review Findings
ExchangeCommitFacet Manual Review Findings
ECF-01M: Inexistent Re-Entrancy Protection
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | ExchangeCommitFacet.sol:L205-L236 |
Description:
The ExchangeCommitFacet::createOfferAndCommit function does not possess a re-entrancy guard modifier despite interacting with external tokens that may re-enter internally.
Impact:
While no active exploitation path exists, it is still advisable to adhere to a security-first approach and implement this mechanism instead of relying on internal-call re-entrancy guard applications.
Example:
205function createOfferAndCommit(206 BosonTypes.FullOffer calldata _fullOffer,207 address _offerCreator,208 address payable _committer,209 bytes calldata _signature,210 uint256 _conditionalTokenId,211 BosonTypes.SellerOfferParams calldata _sellerParams212) external payable override exchangesNotPaused buyersNotPaused sellersNotPaused {213 if (214 _fullOffer.offer.creator == BosonTypes.OfferCreator.Seller &&215 (_sellerParams.collectionIndex != 0 ||216 _sellerParams.royaltyInfo.recipients.length != 0 ||217 _sellerParams.royaltyInfo.bps.length != 0 ||218 _sellerParams.mutualizerAddress != address(0))219 ) revert SellerParametersNotAllowed();220
221 uint256 offerId = prepareOfferForCommit(_fullOffer, _offerCreator, _signature);222
223 if (_fullOffer.condition.method != BosonTypes.EvaluationMethod.None) {224 if (_fullOffer.offer.creator == BosonTypes.OfferCreator.Buyer) {225 addSellerParametersToBuyerOffer(_committer, offerId, _sellerParams);226 }227
228 commitToConditionalOffer(_committer, offerId, _conditionalTokenId);229 } else {230 if (_fullOffer.offer.creator == BosonTypes.OfferCreator.Buyer) {231 commitToBuyerOffer(offerId, _sellerParams);232 } else {233 commitToOffer(_committer, offerId);234 }235 }236}Recommendation:
As the refactor of exchange-handling facets has exposed several internal functions, we believe the overhead of inlining the commitX internal calls that apply the re-entrancy guard again is minimal.
As such, we advise the re-entrancy guard to be applied at the top-level call of ExchangeCommitFacet::createOfferAndCommit properly.
Alleviation (8fccc2716047809e4bc7786c95d346e5c61b2896):
The re-entrancy guard has been applied per our recommendation, utilizing newly declared "unguarded" function implementations that do not apply the re-entrancy guard thus permitting them to be invoked internally.