CultureIndex accepts arbitrary URLs which may lead to Cross-Site Scripting or similar attacks

[c4-bot-9]

Lines of code

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

Vulnerability details

Impact

CultureIndex allows users to upload any images/links which may become future Verbs. However there is no even an enforcement of the URL prefix, or any other validation, as in for example contractURI function. That means, that a malicious user may supply links such as:

• javascript:eval(atob('YWxlcnQoJ3BvYycp')) //(YWxlcnQoJ3BvYycp = alert('poc')) - a malicious script or redirect can be encoded here
• link an SVG image from elsewhere, which may contain a script as well
• try injecting any other malicious links or redirects, falling into web2 security categories such as: XSS or Open redirections

If such image is even present on the CultureIndex website ready to vote, attacker may already be able to attack users who try to visit/view the image.

Impact of injecting malicious script to the page may vary, most likely attacker may try to invoke a script that pops up a malicious approve or other well-known wallet drainer logic or use some sort of popups or UI manipulations to social engineer user to execute harmful actions.

Proof of Concept

Function createPiece has only one validation routine for the metadatas which is called here validateMediaType

the function

    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");
    }

The validation relies only on checking if string is no empty, which is not a security validation at all. Side note: the text inputs may become an attack vector for such manipulations too, but its rather out of contract scope and something that should be validated in this case on the frontend layer. However, validating an URL may be more tricky, since it still can point to an external resource with a redirection while being correctly formatted.

Tools Used

Manual approach

Recommended Mitigation Steps

The simplest approach could be to force the URL to start with ipfs:// so user-provided string will be appended to ipfs:// which excludes protocol handler tricks such as javascript: or redirections. For potential breaking of the URI scheme a validation of only-alphanumeric characters in the URI may come in handy.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as duplicate of #167

[c4-sponsor]

rocketman-21 (sponsor) confirmed

[c4-judge]

MarioPoneder changed the severity to 3 (High Risk)

[MarioPoneder]

Partial credit due to having found 1/2 impact of primary issue.

[c4-judge]

MarioPoneder marked the issue as partial-50