Submitted by immeas, also found by btk, dy, 7siech, and Myd
The royalty allocation within the protocol works like this:
First, an admin sets the split between team and artist. Here it is validated that the artist and team allocations together fill up 100%:
MinterContract::setPrimaryAndSecondarySplits:
The artist can then propose a split between artist addresses, where it is validated that the different artist addresses together add up to the total artist allocation:
MinterContract::proposePrimaryAddressesAndPercentages:
Then, also when paid out, there is a validation that the split between team and artist allocation adds up:
MinterContract::payArtist:
The issue is that here, it compares to the artist allocation from artist/team allocations. When the actual amount paid out is calculated, it uses the proposed (and accepted) artist address percentages:
MinterContract::payArtist:
Hence, there is a possibility that up to double the intended amount can be paid out, imagine this scenario:
They then call payArtist with the 100% team allocation. This will pass the check as artistPercentage will be 0. However, the artist payouts will use the already proposed artist address allocations. Hence, double the amount will be paid out.
An admin can maliciously, or by mistake, manipulate the percentage allocation for a collections royalty to pay out up to double the amount of royalty. This could make it impossible to payout royalty later, as this steals royalty from other collections.
PoC using foundry. Save file in hardhat/test/MinterContractTest.t.sol, also needs forge-std in hardhat/lib:
Consider verifying that the artist allocations add up to the artist percentage when calling payArtist.
Invalid Validation
a2rocket (NextGen) disputed and commented via duplicate issue #1550:
0xsomeone (judge) commented:
MrPotatoMagic (warden) commented:
0xsomeone (judge) commented:
