Omniscia Boson Protocol Audit
SellerBase Manual Review Findings
SellerBase Manual Review Findings
SBE-01M: Potential of Race Condition in Seller Creation
Type | Severity | Location |
---|---|---|
Logical Fault | ![]() | SellerBase.sol:L32-L86 |
Description:
The createSellerInternal
function as well as where it is called apply no access control to the caller. Given that the seller creation system strictly associates user types with a single seller ID, it is possible for a race condition to manifest by detecting the creation of a seller account and adjusting the treasury
address to a different one causing an unsuspecting user to never receive the funds they are owed for a sale.
Example:
32function createSellerInternal(33 Seller memory _seller,34 AuthToken calldata _authToken,35 VoucherInitValues calldata _voucherInitValues36) internal {37 //Check active is not set to false38 require(_seller.active, MUST_BE_ACTIVE);39
40 //Admin address or AuthToken data must be present. A seller can have one or the other41 require(42 (_seller.admin == address(0) && _authToken.tokenType != AuthTokenType.None) ||43 (_seller.admin != address(0) && _authToken.tokenType == AuthTokenType.None),44 ADMIN_OR_AUTH_TOKEN45 );46
47 //Check that the addresses are unique to one seller Id, accross all roles. These addresses should always be checked. Treasury is not checked48 require(49 protocolLookups().sellerIdByOperator[_seller.operator] == 0 &&50 protocolLookups().sellerIdByOperator[_seller.clerk] == 0 &&51 protocolLookups().sellerIdByAdmin[_seller.operator] == 0 &&52 protocolLookups().sellerIdByAdmin[_seller.clerk] == 0 &&53 protocolLookups().sellerIdByClerk[_seller.operator] == 0 &&54 protocolLookups().sellerIdByClerk[_seller.clerk] == 0,55 SELLER_ADDRESS_MUST_BE_UNIQUE56 );57
58 //Do other uniqueness checks based on auth type59 if (_seller.admin == address(0)) {60 //Check that auth token is unique to this seller61 require(62 protocolLookups().sellerIdByAuthToken[_authToken.tokenType][_authToken.tokenId] == 0,63 AUTH_TOKEN_MUST_BE_UNIQUE64 );65 } else {66 //check that the admin address is unique to one seller Id, accross all roles67 require(68 protocolLookups().sellerIdByOperator[_seller.admin] == 0 &&69 protocolLookups().sellerIdByAdmin[_seller.admin] == 0 &&70 protocolLookups().sellerIdByClerk[_seller.admin] == 0,71 SELLER_ADDRESS_MUST_BE_UNIQUE72 );73 }74
75 // Get the next account Id and increment the counter76 uint256 sellerId = protocolCounters().nextAccountId++;77 _seller.id = sellerId;78 storeSeller(_seller, _authToken);79
80 // create clone and store its address cloneAddress81 address voucherCloneAddress = cloneBosonVoucher(sellerId, _seller.operator, _voucherInitValues);82 protocolLookups().cloneAddress[sellerId] = voucherCloneAddress;83
84 // Notify watchers of state change85 emit SellerCreated(sellerId, _seller, voucherCloneAddress, _authToken, msgSender());86}
Recommendation:
We advise the seller creation flow to apply some form of access control to the caller or sanitization of the input arguments to ensure that an account cannot be created for other users and thus disallowing this type of fraud attempt from ever being feasible on-chain.
Alleviation (68ebb15f14b12c7e64c233ff00363398a127a71e):
The createSellerInternal
function now mandates that the _seller.operator
and _seller.clerk
values (as well as, conditionally, the _seller.admin
value) all represent the current transaction sender thus preventing accounts to be created for other users. To ensure the system still permits an account to be updated in relation to those values, an opt-in system has been introduced to the SellerHandlerFacet
implementation that treats updates as "pending" until the recipient of the role "opts-in" to the upgrade manually, ensuring accounts and responsibilities cannot be offloaded to other addresses without confirmation of receipt. As a result of this workflow adjustment, we consider this exhibit alleviated in full.