Input Validation for createPiece Function

[c4-bot-10]

Lines of code

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

Vulnerability details

Potential Risk: The createPiece function in the CultureIndex contract takes two parameters: metadata and creatorArray. While the function calls the validateCreatorsArray and validateMediaType functions to validate the input data, it does not verify whether the provided pieceId is already used or whether it falls within a valid range. This could potentially lead to issues if the same pieceId is reused or if it exceeds the expected range.

Proof of Concept (PoC): Consider a scenario where a malicious user attempts to reuse an existing pieceId:

// Existing piece with pieceId 1 ArtPieceMetadata memory existingMetadata; CreatorBps[] memory existingCreators;

// Attempt to create a new piece with the same pieceId createPiece(existingMetadata, existingCreators);

In this PoC, a malicious user attempts to create a new piece with the same pieceId as an existing one. The createPiece function does not have a check to prevent this, potentially leading to unexpected behavior.

Recommended Mitigation Steps: To mitigate the risk of reusing existing pieceId values or exceeding valid ranges, consider adding a validation check for the pieceId. Here's a recommended solution:

function createPiece( ArtPieceMetadata calldata metadata, CreatorBps[] calldata creatorArray ) public returns (uint256) { uint256 creatorArrayLength = validateCreatorsArray(creatorArray);

// Validate the media type and associated data
validateMediaType(metadata);

uint256 pieceId = _currentPieceId++;

// Add a validation check for pieceId
require(pieceId < MAX_PIECE_ID, "Invalid pieceId");

// ... rest of the function ...

emit PieceCreated(pieceId, msg.sender, metadata, newPiece.quorumVotes, newPiece.totalVotesSupply);

// Emit an event for each creator
for (uint i; i < creatorArrayLength; i++) {
    emit PieceCreatorAdded(pieceId, creatorArray[i].creator, msg.sender, creatorArray[i].bps);
}

return newPiece.pieceId;

}

In this solution:

• We've added a check to ensure that pieceId is within a valid range (less than MAX_PIECE_ID).
• This change helps prevent the reuse of existing pieceId values and ensures that the pieceId is within an expected range.

By implementing this solution, you can enhance the input validation of the createPiece function and reduce the risk of issues related to pieceId values.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #688

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Insufficient proof

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Insufficient proof