Lines of code
<https://github.com/code-423n4/2023-10-nextgen/tree/main/smart-contracts/MinterContract.sol#L418&gt;

Vulnerability details

Impact

The vulnerability in the payment mechanism of the smart contract significantly impacts the protocol’s functionality. The root cause of the vulnerability is that, despite the README stating an invariant that “Payments can only be made when royalties are set, the artist proposes addresses and percentages, and an admin approves them,” poorly managed status control allows the admin to accept addresses and percentages without requiring an artist’s proposal. This oversight breaks the stated invariants and has several negative consequences.

// File: README.md
178:- Payments can only be made when royalties are set, the artist proposes addresses and percentages, and an admin approves them. // <= FOUND

Proof of Concept

To demonstrate this vulnerability, an admin can accept addresses and percentages without requiring an artist’s proposal, resulting in unauthorized royalty payments to the team wallet.

1. The admin, due to the poorly managed status control, can accept addresses and percentages without waiting for the artist to make a proposal.

// File: smart-contracts/MinterContract.sol
408:    function acceptAddressesAndPercentages(uint256 _collectionID, bool _statusPrimary, bool _statusSecondary) public FunctionAdminRequired(this.acceptAddressesAndPercentages.selector) {// <= FOUND: Payments can be made skipping royalties set and the artist proposes.
409:        collectionArtistPrimaryAddresses[_collectionID].status = _statusPrimary;
410:        collectionArtistSecondaryAddresses[_collectionID].status = _statusSecondary;
411:    }

2. This allows the admin to proceed to obtain all royalties to the team wallet via the payArtist function.

// File: smart-contracts/MinterContract.sol
415:    function payArtist(uint256 _collectionID, address _team1, address _team2, uint256 _teamperc1, uint256 _teamperc2) public FunctionAdminRequired(this.payArtist.selector) {
...
418:        require(collectionRoyaltiesPrimarySplits[_collectionID].artistPercentage + _teamperc1 + _teamperc2 == 100, "Change percentages");// <= FOUND: artistPercentage is 0 if skipped previous steps. Admin can chose _teamperc1 + _teamperc2 = 100% to obtain all royalties
        ...
444:    }

3. The protocol’s payment functionality is compromised, and the invariant is not upheld.
4. Further, this violation can lead to other scenarios, such as the admin resetting the artist/team split even after approval or an artist front-running the admin’s approval with a different proposal against the admin’s intention.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this issue, it is recommended to extend the status control mechanism into multiple stages, such as “INIT,” “PERCENTAGE_SPLIT_SET,” “ARTIST_PROPOSED,” and “PERCENTAGE_ACCEPTED,” and control the state flow properly. This will ensure that payments are made only when royalties are set, the artist proposes addresses and percentages, and an admin approves them, as specified in the README.

Assessed type

Other

---

The text was updated successfully, but these errors were encountered:

All reactions