Validate Creators Array

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L179

Vulnerability details

Potential Risk: The validateCreatorsArray function in the CreatorUtils contract performs input validation for the creatorArray parameter, which is an array of CreatorBps structs. While the function checks the individual elements of the array for valid addresses and ensures that the sum of bps values equals 10,000, it does not check for duplicate addresses in the array. This could lead to a situation where multiple creators have the same address, potentially causing unintended behavior.

Proof of Concept (PoC): Consider a scenario where an attacker provides a 'creatorArray' with duplicate addresses:

CreatorBps[] memory maliciousCreators = new CreatorBps;

// Duplicate address for two creators maliciousCreators[0] = CreatorBps(address(creator1), 5000); maliciousCreators[1] = CreatorBps(address(creator1), 5000);

// Attempt to validate the malicious creators array validateCreatorsArray(maliciousCreators);

In this PoC, the attacker creates an array of 'CreatorBpswith' the same address for two creators. The 'validateCreatorsArray' function does not detect this as an issue, even though it should be considered invalid.

Recommended Mitigation Steps: To improve the input validation for the creatorArray, you should consider adding validation checks to ensure that there are no duplicate addresses in the array.

Here's a recommended solution:

function validateCreatorsArray(CreatorBps[] calldata creatorArray) internal pure returns (uint256) { uint256 creatorArrayLength = creatorArray.length; // Require that creatorArray is not more than MAX_NUM_CREATORS to prevent gas limit issues require(creatorArrayLength <= MAX_NUM_CREATORS, "Creator array must not be > MAX_NUM_CREATORS");

uint256 totalBps;
mapping(address => bool) addressSet; // Keep track of unique addresses

for (uint256 i = 0; i < creatorArrayLength; i++) {
    require(creatorArray[i].creator != address(0), "Invalid creator address");
    require(!addressSet[creatorArray[i].creator], "Duplicate creator address");
    addressSet[creatorArray[i].creator] = true; // Mark the address as seen
    totalBps += creatorArray[i].bps;
}

require(totalBps == 10_000, "Total BPS must sum up to 10,000");

return creatorArrayLength;

}

In this solution:

• We've added a 'mapping called 'addressSet' to keep track of unique addresses encountered in the 'creatorArray'.
• Before processing each element, we check if the address has been seen before (indicating a duplicate) and revert the transaction if a duplicate is detected.
• This change ensures that the 'creatorArray' does not contain duplicate addresses, enhancing the security of the input validation.

By implementing this solution, you can prevent the use of duplicate addresses in the 'creatorArray' and improve the reliability of your input validation in the validateCreatorsArray' function.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[raymondfam]

Inconsequential scenario.

[c4-pre-sort]

raymondfam marked the issue as duplicate of #688

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Insufficient proof