Canto Identity Subprotocols contest - Haipls's results

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-bio-protocol/src/Bio.sol#L121

Vulnerability details

Impact

• Generate invalid svg Users may use forbidden or incorrect characters for SVG. Which will generate invalid svg As a result, you will need to mint a new NFT with the correct bio. Otherwise, the image cannot be displayed without pre-processing and correction

Proof of Concept

The user can provide one of the invalid characters for SVG/XML (eg &, <, >, etc...) In the mint method, there is no validation for the correctness of the biography.

Example:

1. User A mint with next bio

Jane Doe, born in LA in 20, is an acclaimed actress & director. She starred in the hit series 'City Light' and directed the award-winning film 'Hidden Shadows'. Jane is also an animal rights activist.

2. WebSite call tokenURI for minted NFT and receive:

{
  "name": "Bio #1",
  "description": "Jane Doe, born in LA in 20, is an acclaimed actress & director. She starred in the hit series 'City Light' and directed the award-winning film 'Hidden Shadows'. Jane is also an animal rights activist.",
  "image": "data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHByZXNlcnZlQXNwZWN0UmF0aW89InhNaW5ZTWluIG1lZXQiIHZpZXdCb3g9IjAgMCA0MDAgMTAwIj48c3R5bGU+dGV4dCB7IGZvbnQtZmFtaWx5OiBzYW5zLXNlcmlmOyBmb250LXNpemU6IDEycHg7IH08L3N0eWxlPjx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBkb21pbmFudC1iYXNlbGluZT0ibWlkZGxlIiB0ZXh0LWFuY2hvcj0ibWlkZGxlIj48dHNwYW4geD0iNTAlIiBkeT0iMjAiPkphbmUgRG9lLCBib3JuIGluIExBIGluIDIwLCBpcyBhbiBhY2NsYWk8L3RzcGFuPjx0c3BhbiB4PSI1MCUiIGR5PSIyMCI+bWVkIGFjdHJlc3MgJiBkaXJlY3Rvci4gU2hlIHN0YXJyZWQgaW4gdDwvdHNwYW4+PHRzcGFuIHg9IjUwJSIgZHk9IjIwIj5oZSBoaXQgc2VyaWVzICdDaXR5IExpZ2h0JyBhbmQgZGlyZWN0ZWQgPC90c3Bhbj48dHNwYW4geD0iNTAlIiBkeT0iMjAiPnRoZSBhd2FyZC13aW5uaW5nIGZpbG0gJ0hpZGRlbiBTaGFkb3dzJy48L3RzcGFuPjx0c3BhbiB4PSI1MCUiIGR5PSIyMCI+IEphbmUgaXMgYWxzbyBhbiBhbmltYWwgcmlnaHRzIGFjdGl2aXN0LjwvdHNwYW4+PC90ZXh0Pjwvc3ZnPg=="
}

3. WebSite gets image field from the result and sets src attribute of img to display bio
4. The user does not see the picture. since svg invalid

Tools Used

• Manual review
• https://en.wikipedia.org/wiki/Valid_characters_in_XML

Recommended Mitigation Steps

• Add validation of the passed bio
• Wrap the text so that these characters are not effect

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-bio-protocol/src/Bio.sol#L121 https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-bio-protocol/src/Bio.sol#L43

Vulnerability details

I don't know if this is in the scope, but it will be useful

Impact

• The possibility of adding incorrect photos to the biography
• Can add bad links in SVG (when clicking on image of bio)
• The user can insert malicious code that can then be executed on WebSites for other users where data on NFT biographies is called from SC

Proof of Concept

1. The user can fill in any information as a biography
2. Biography is generated SVG where the provided biography is placed in the middle
3. SVG:
   1. Embedding JavaScript code within the <script> tag inside an SVG file.
   2. Using SVG event attributes, like onclick, onmouseover, or onload, to execute JavaScript code.
   3. Employing SVG-specific tags like <animate>, <set>, or <handler> to execute malicious code.
   4. Exploiting XML-based features, such as external entity references, to perform XML External Entity (XXE) attacks.
4. The site that uploads information on NFT processes it by direct inclusion or in another incorrect way

Examples of bad biographies

Load an invalid photo

"<image href='http://surl.li/fphve'/>"

Execute JS

"<script>console.log('WOW')</script>"

But actually, it's not so bad because there are 2 things that make it difficult to use:

1. Limiting the length of one line to 40 characters
2. Use of data:image/svg+xml;base64 type

The first makes it difficult to work with long links. If data is used and it is not decoded, then the code will not be executed, but only until the user clicks on the photo link...

Everything changes if the data is decoded and inserted as svg... Then the code will be executed and injected

example decode svg from tokenURI for test:

<svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 100">
   <text x="50%" y="50%" dominant-baseline="middle" text-anchor="middle"><tspan x="50%" dy="20"><script>console.log('WOW')</script></tspan></text>
</svg>

Tools Used

• Manual review
• SVG Inject docs

Recommended Mitigation Steps

• Add validation
• Warn those who will use the protocol

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-bio-protocol/src/Bio.sol#L43

Vulnerability details

Impact

• Difficulty to understand
• The cost of the call
• Terrible line breaks and word wrapping
• Excessive complexity
• no > UTF-8 support

Proof of Concept

The task of the tokenURI method is to return a json object with a generated svg where the biography up to 200 bytes long is divided by 40 per line. This was implemented by iterating through each character of the bio, with a large number of operations. Which makes it difficult to understand, increases the possibility of making mistakes, the cost of a method call, etc.

I recommend transferring the line break to the shoulders of SVG tags and styles, which will simplify and make the code many times cheaper, and get rid of potential errors.

An example is given below:

---

Old Flow

Code

Actual code code +-43 lines, 14 variables, a lot differens operations

Old SVG format

  <svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 100">
    <style>
      text {
        font-family: sans-serif;
        font-size: 12px;
      }
    </style>
    <text x="50%" y="50%" dominant-baseline="middle" text-anchor="middle">
      <tspan x="50%" dy="20">Jane Doe, born in LA in 20, is an acclai</tspan>
      <tspan x="50%" dy="20">med actress director. She starred in t</tspan>
      <tspan x="50%" dy="20">he hit series 'City Light' and directed </tspan>
      <tspan x="50%" dy="20">the award-winning film 'Hidden Shadows'.</tspan>
      <tspan x="50%" dy="20"> Jane is also an animal rights activist.</tspan>
    </text>
  </svg>

Old SVG View (set additional 400px:200px):

New Flow:

SVG format

      <svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMinYMin meet" viewBox="0 0 400 200">
        <style>
          .c {
            display: flex;
            align-items: center;
            justify-content: center;
            height: 100%;
          }
          .bio {
            font-family: sans-serif;
            font-size: 12px;
            max-width: 34ch;
            line-height: 20px;
            hyphens: auto;
          }
        </style>
        <foreignObject width="100%" height="100%">
          <div class="c" xmlns="http://www.w3.org/1999/xhtml">
            <div class="bio">Jane Doe, born in LA in 20, is an acclaimed actress director. She starred in the hit series 'City Light' and directed the award-winning film 'Hidden Shadows'. Jane is also an animal rights activist.</div>
          </div>
        </foreignObject>
      </svg>

New SVG view from example (set additional 400px:200px):

This is just an example. The point is to use svg, css tags in order to reduce the complexity of the code many times

We can see that there is more plain text.

But now we don't have to worry about the following things:

• Split rows
• Support UTF-8
• Correct transfer UTF-8 characters to new line etc
• Avoiding potential problems
• Most cheaper the gas
• Complexity
• etc..

Now we need to return only the template and insert the bio in the center of the template Code will look like:

    function tokenURI(uint256 _id) public view override returns (string memory) {
        if (_ownerOf[_id] == address(0)) revert TokenNotMinted(_id);
        string memory bioText = bio[_id];
        bytes memory imageBytes =  bytes(
            string.concat(
                '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 400 200"><style>.c{display:flex;align-items:center;justify-content:center;height:100%;}.bio{font-family:sans-serif;font-size:12px;max-width:34ch;line-height:20px;hyphens:auto;}</style><foreignObject width="100%" height="100%"><div class="c" xmlns="http://www.w3.org/1999/xhtml"><div class="bio">',
                bioText,
                "</div></div></foreignObject></svg>"
            )
        );
        string memory json = Base64.encode(
            bytes(
                string.concat(
                    '{"name": "Bio #',
                    LibString.toString(_id),
                    '", "description": "',
                    bioText,
                    '", "image": "data:image/svg+xml;base64,',
                    Base64.encode(imageBytes),
                    '"}'
                )
            )
        );
        return string.concat("data:application/json;base64,", json);
    }

We see a colossal simplification, With the avoidance of many moments

• Of the problems is setting the styles correctly, because I just gave an example

Compare this with actual code code the method became cheaper by an order of magnitude, the complexity was reduced to a minimum. Now there are only 2 limitations for the biography, these are: length, and limitations to the XML text

Tools Used

• Manual review

Recommended Mitigation Steps

• Shift responsibility to SVG