code-423n4 / 2023-10-nextgen-findings

5 stars 3 forks source link

No function to distribute secondary sales royalties split. #1973

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L95-L95 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L109-L109

Vulnerability details

Impact

The minter contract currently does not have a way to reliably share the secondary sales revenue for the proposed addresses and splits.

Proof of Concept

Contract : NextGenMinterContract

The minter contract uses collectionArtistSecondaryAddresses and collectionRoyaltiesSecondarySplits mappings to track the addresses and percentage splits for revenue obtained from secondary royalties.

The contract uses payArtist() function to split the primary sales revenue.

However, the contract has no such function to dispatch the secondary sales revenue trustably.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Employ an access controlled function to split secondary royalties reliably.

Assessed type

Other

c4-pre-sort commented 10 months ago

141345 marked the issue as duplicate of #1105

c4-judge commented 10 months ago

alex-ppg marked the issue as not a duplicate

c4-judge commented 10 months ago

alex-ppg marked the issue as primary issue

alex-ppg commented 10 months ago

The Warden specifies that a discrepancy in the system occurs whereby a secondary royalty system has been set in place in terms of data but remains unutilized within the NextGen system.

The NextGen documentation is also not helpful in this regard given that it is contradictory (i.e. acceptPrimaryAddressesAndPercentages function does not exist, and the payArtist documentation is insufficient).

I would like to invite the Sponsor to review this. If the secondary royalties are meant to be a future feature or should be cleaned up, this will be QA. Otherwise, this will be a medium-severity issue as it relates to functionality that a user would expect but is not actually implemented.

c4-sponsor commented 10 months ago

a2rocket (sponsor) disputed

a2rocket commented 10 months ago

secondary splits refer to the royalties that are collected when a token is sold on a marketplace. The collected royalties will be tracked from a different tool which is out of scope in this audit and will be distributed to the artists based on the addresses that an artist proposed on the NextGen minter contract.

alex-ppg commented 10 months ago

As the Sponsor has clarified, these data points are meant to be utilized externally by an off-chain out-of-scope tool that NextGen will create / already has. As such, for the purposes of the contest these are meant to be simple data entries and thus behave as expected rendering this exhibit invalid.

c4-judge commented 10 months ago

alex-ppg marked the issue as unsatisfactory: Invalid