[SBE-01M] Incorrect Default Royalty Recipient Initialization

[zajck]

SBE-01M: Incorrect Default Royalty Recipient Initialization

Type	Severity	Location
Logical Fault		SellerBase.sol:L99

Description:

The SellerBase::createSellerInternal function will initialize the default royalty recipient of the seller (i.e. address(0)) without setting its royaltyRecipientIndexBySellerAndRecipient entry. As a result, the default recipient can be re-added via the SellerHandlerFacet::addRoyaltyRecipients and SellerHandlerFacet::updateRoyaltyRecipients functions incorrectly, causing the overall royalty system of the contract to misbehave.

Impact:

It is presently possible for the treasury to exist twice as a royalty recipient for a particular seller which we consider a major invariant breach of the protocol.

Example:

RoyaltyRecipient[] storage royaltyRecipients = lookups.royaltyRecipientsBySeller[sellerId];
RoyaltyRecipient storage defaultRoyaltyRecipient = royaltyRecipients.push();
// We don't store the defaultRoyaltyRecipient.wallet, since it's always the trasury
// We don't store the defaultRoyaltyRecipient.externalId, since the default recipient is always the treasury
defaultRoyaltyRecipient.minRoyaltyPercentage = _voucherInitValues.royaltyPercentage;

Recommendation:

We advise the royaltyRecipientIndexBySellerAndRecipient entry of the address(0) to be properly maintained, preventing it from being re-added and thus ensuring that the treasury recipient exists only once.