code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

Insufficient input Sanitization could lead to XSS, CSRF on Users Browsers #729

Closed c4-bot-4 closed 10 months ago

c4-bot-4 commented 10 months ago

Lines of code

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

Vulnerability details

Impact

The artpiece content is only validated for the length. Due To the fact that creating a new piece would only cost gas and there is no additional fees. It is possible for attackers to flood the system with malicious content. And if on the sites that will showcase the nft there is no sufficient formatting when showcasing the NFT. This could lead to the malicious user executing malicious javascript code on the victim (the user who opened the website) and potentially also interacting with the victims wallet.

Proof of Concept

The ArtpieceMetadata is only validated for it's content length. Meaning it is possible to store any nft with malicious content on the smart contract

    function validateMediaType(ArtPieceMetadata calldata metadata) internal pure {
        require(uint8(metadata.mediaType) > 0 && uint8(metadata.mediaType) <= 5, "Invalid media type");

        if (metadata.mediaType == MediaType.IMAGE)
            require(bytes(metadata.image).length > 0, "Image URL must be provided");
        else if (metadata.mediaType == MediaType.ANIMATION)
            require(bytes(metadata.animationUrl).length > 0, "Animation URL must be provided");
        else if (metadata.mediaType == MediaType.TEXT)
            require(bytes(metadata.text).length > 0, "Text must be provided");
    }

Example malicious content:

<script> alert("XSS") </script>

Tools Used

.

Recommended Mitigation Steps

We could recommend checking for a maximum length of the stored content and also may be sanitizing the Artpiece Metadata content Example

function sanitizeString(input) {
    // Replace characters that have special meaning in HTML
    const sanitizedString = input.replace(/</g, "&lt;").replace(/>/g, "&gt;");

    // Additional sanitation steps based on your specific needs

    return sanitizedString;
}

Assessed type

Invalid Validation

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #59

c4-judge commented 9 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

MarioPoneder marked the issue as grade-c