c4-submissions commented 1 year ago

Lines of code

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

Vulnerability details

Impact

When an artist adds the primary and secondary addresses as well as their percentages, they need the admin to confirm it. That means that the contract admin wants the limit the values the artist can use. But, with the current setup, the artist can game the system.

They first propose values that are acceptable. After that, the admin sends the confirmation transaction. The artist then frontruns the admin's transaction and proposes new values. The new values are confirmed and the admin isn't able to undo those changes.

Proof of Concept

After the artist proposes the addresses and their percentages, they can call the function again to change the proposed values:

    function proposePrimaryAddressesAndPercentages(uint256 _collectionID, address _primaryAdd1, address _primaryAdd2, address _primaryAdd3, uint256 _add1Percentage, uint256 _add2Percentage, uint256 _add3Percentage) public ArtistOrAdminRequired(_collectionID, this.proposePrimaryAddressesAndPercentages.selector) {
        require (collectionArtistPrimaryAddresses[_collectionID].status == false, "Already approved");
        require (_add1Percentage + _add2Percentage + _add3Percentage == collectionRoyaltiesPrimarySplits[_collectionID].artistPercentage, "Check %");
        collectionArtistPrimaryAddresses[_collectionID].primaryAdd1 = _primaryAdd1;
        collectionArtistPrimaryAddresses[_collectionID].primaryAdd2 = _primaryAdd2;
        collectionArtistPrimaryAddresses[_collectionID].primaryAdd3 = _primaryAdd3;
        collectionArtistPrimaryAddresses[_collectionID].add1Percentage = _add1Percentage;
        collectionArtistPrimaryAddresses[_collectionID].add2Percentage = _add2Percentage;
        collectionArtistPrimaryAddresses[_collectionID].add3Percentage = _add3Percentage;
        collectionArtistPrimaryAddresses[_collectionID].status = false;
    }

    // function to propose secondary addresses and percentages for each address

    function proposeSecondaryAddressesAndPercentages(uint256 _collectionID, address _secondaryAdd1, address _secondaryAdd2, address _secondaryAdd3, uint256 _add1Percentage, uint256 _add2Percentage, uint256 _add3Percentage) public ArtistOrAdminRequired(_collectionID, this.proposeSecondaryAddressesAndPercentages.selector) {
        require (collectionArtistSecondaryAddresses[_collectionID].status == false, "Already approved");
        require (_add1Percentage + _add2Percentage + _add3Percentage == collectionRoyaltiesSecondarySplits[_collectionID].artistPercentage, "Check %");
        collectionArtistSecondaryAddresses[_collectionID].secondaryAdd1 = _secondaryAdd1;
        collectionArtistSecondaryAddresses[_collectionID].secondaryAdd2 = _secondaryAdd2;
        collectionArtistSecondaryAddresses[_collectionID].secondaryAdd3 = _secondaryAdd3;
        collectionArtistSecondaryAddresses[_collectionID].add1Percentage = _add1Percentage;
        collectionArtistSecondaryAddresses[_collectionID].add2Percentage = _add2Percentage;
        collectionArtistSecondaryAddresses[_collectionID].add3Percentage = _add3Percentage;
        collectionArtistSecondaryAddresses[_collectionID].status = false;
    }

When the admin confirms the proposed values there are no additional checks:

    function acceptAddressesAndPercentages(uint256 _collectionID, bool _statusPrimary, bool _statusSecondary) public FunctionAdminRequired(this.acceptAddressesAndPercentages.selector) {
        collectionArtistPrimaryAddresses[_collectionID].status = _statusPrimary;
        collectionArtistSecondaryAddresses[_collectionID].status = _statusSecondary;
    }

Tools Used

none

Recommended Mitigation Steps

In the acceptAddressesAndPercentages() function, include the expected values as parameters and check the current values against them. If they don't match, the function should revert. That will prevent the artist from frontrunning that tx.

Assessed type

Other

c4-pre-sort commented 1 year ago

141345 marked the issue as sufficient quality report

c4-pre-sort commented 1 year ago

141345 marked the issue as primary issue

c4-sponsor commented 1 year ago

a2rocket (sponsor) disputed

a2rocket commented 1 year ago

artist can propose addresses and once those addresses are accepted they cannot change. Until an admin approves addresses artists can change them.

c4-judge commented 11 months ago

alex-ppg marked the issue as duplicate of #722