ReceiptRenderer SVG code is incorrect and makes text illegible

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/ReceiptRenderer.sol#L100-L114

Vulnerability details

In ReceiptRenderer.sol the generated SVG in generateSVG has both texts (RabbitHole Quest # and RabbitHole Quest Receipt #) rendered on top of each other, making the text completely illegible.

Impact

SVG rendering of receipts will be illegible for users.

Proof of Concept

Put the SVG code in a renderer: you will see that both texts are on top of each other.
The SVG looks like this (using questId_ = 1, and tokenId_ = 1):

Tools Used

SVG Viewer

Recommended Mitigation Steps

Use the SVG tag tspan for each row of text. Correct code:

/// @dev generates the on-chain SVG for an ERC-721 token ID
/// @param tokenId_ The token id to generate the svg for
/// @param questId_ The questId tied to the tokenId
/// @return encoded JSON for an SVG image
function generateSVG(uint tokenId_, string memory questId_) public pure returns (string memory) {
    bytes memory svg = abi.encodePacked(
        '<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 350 350">',
        '<style>.base { fill: white; font-family: serif; font-size: 14px; }</style>',
        '<rect width="100%" height="100%" fill="black" />',
        '<text y="40%">',
        '<tspan x="50%" dy=".6em" class="base" text-anchor="middle">RabbitHole Quest #',
        questId_,
        '</tspan>',
        '<tspan x="50%" dy="1.2em" class="base" text-anchor="middle">RabbitHole Quest Receipt #',
        tokenId_,
        '</tspan>',
        '</text>',
        '</svg>'
    );
    return string(abi.encodePacked('data:image/svg+xml;base64,', Base64.encode(svg)));
}

Result:

[c4-sponsor]

waynehoover marked the issue as disagree with severity

[waynehoover]

Just text display of an NFT, this rendering detail has and will change on final deployment.

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)

[kirk-baird]

This would not result in a direct loss of funds and is a valid QA issue.

[c4-judge]

kirk-baird marked the issue as grade-b