Cross-Site Scripting:DOM - Issue

[karthikdav]

Sending unvalidated data to a web browser can result in the browser executing malicious code. This was reported by Fortify scanner since I'm using minified version I'm not able to get the exact line.

[knsv]

Thx, we should look at this

[knsv]

Hi again. In what context are you getting this error.

Mermaid itself takes elements on the wepage compiles them to svg element and inserts the svg in the DOM. Generally when talking about cross site scripting there is a input field or similar in which an external user could add javascript code that the unprepared web site then puts in its DOM where it starts to execute.

I am certain mermaid would not allow javascript in the diagram code (even if would be a cool but unsafe feature).

[karthikdav]

Even I'm not sure how that'll happen, I even cross checked the security scan report. Even I'm little skeptical about the code where it's happening

[darrenmothersele]

You can confirm this by going to the editor and entering the code given on the NPM advisory https://www.npmjs.com/advisories/751

[rogeralmeida]

Hey, I was checking the code and I believe that a possible solution to this problem would be:

/**
* MermaidAPI.js line 239
*/
   function parse (text) {
  //SANITIZE INPUT!
  const graphType = utils.detectType(text)
  let parser
  ...

The testing for this would be interesting though. If you guys believe this would be an elegant solution I can put sometime to try to fix it in the next 48 hours and open a PR

[rogeralmeida]

I spoke to a senior security specialist yesterday about this, thanks @mario-areias. After understanding better the situation, I don't think my above suggestion is valid anymore. It is unfair to tag mermaid as unsafe for Cross Site Scripting. The attack would happen if the attacker gain access to the input method. AFAIK mermaid has two input methods: content in a html page and via API.

Content in an HTML page

If the attacker gain privileged access to the HTML page, than the attack is already executing the XSS. The attacker doesn't need to use mermaid to gain access as he/she would already have the access.

Via API

Because mermaid offers an API, that can be used to receive the input from anywhere, then yes, this can be a compromise of the security. In this case have to implement a counter measure. I tried to simulate the attack via API and the for the simple case that I implemented the API accepted the input as valid. See below:

it('should prevent XSS injection', function () {
      expect(() => mermaidAPI.parse('graph TD;A["<img src=invalid onerror=alert(\'XSS\')></img>"]')).toThrow()
    })

The result is:

 FAIL  src/mermaidAPI.spec.js
  ● when using mermaidAPI and  › checking validity of input  › should prevent XSS injection

    expect(function).toThrow(undefined)

    Expected the function to throw an error.
    But it didn't throw anything.

      43 |     })
      44 |     it('should prevent XSS injection', function () {
    > 45 |       expect(() => mermaidAPI.parse('graph TD;A["<img src=invalid onerror=alert(\'XSS\')></img>"]')).toThrow()
         |                                                                                                      ^
      46 |     })
      47 |   })
      48 | })

      at Object.toThrow (src/mermaidAPI.spec.js:45:102)

Test Suites: 1 failed, 9 passed, 10 total
Tests:       1 failed, 267 passed, 268 total
Snapshots:   0 total
Time:        4.278s

In this case, one suggestion from @mario-areias is to scape the output, e. g., replace all < and > with < and > respectively.
I'm not familiar with the code yet to know if this is easy or hard to implement.

Conclusion

If you came here because you want to remove that annoying critical vulnerability from you npm audit report:

• If your input is the HTML page you are safe for now.
• if you use the API, we still have to prove that mermaid is safe.

[knsv]

Hi!

Thanks for the anlysis, @rogeralmeida a PR for this would be great.

One thing to consider is that in som cases tags are valid. <br/> for instance is allowed. Another thing to consider is that if the characters are replaced where you suggest this will affect the parsing of the code and subsequencly the grammars (jison files) would need to be updated to look for < and > instead.

The other way to do this is to sanitize after the parsing. The parsing results in a an object containing the data of the graph later used for rendering like an array of vertices etc. To sanitizing after the parsing would perhaps be easier in some sense but not as elegant.

You wound not need to bother with the grammars but the sanitizing would be needed for each diagram type. I think I would go for your suggested route.

[knsv]

@rogeralmeida I added this to the 8.2.0 project. Let me know if you want me to assign it to you.

[rogeralmeida]

OK, I will give it a go and get back to you soon.

[knsv]

Relevant examples fro duplicate issue in #869.

Hi, I found XSS issues in mermaid. This affects all the projects that use mermaid.

There are three different ways to trigger.

The first one:

graph TD
B --> C{<script src=https://www.google-analytics.com/gtm/js?id=GTM-TQ6RV7G ></script>}

The second one:

graph LR;
    A-->B;
    click B callback "<script src=https://www.google-analytics.com/gtm/js?id=GTM-TQ6RV7G ></script>"

The third one(needs click, both nodes will work):

graph LR;
    alert`md5_salt`-->B;
    click alert`md5_salt` eval "Tooltip for a callback"
    click B "javascript:alert`salt`" "This is a tooltip for a link"

Loading

Here is an example that affects other projects which using mermaid.
hackmdio/codimd#1233

And all above three payload would work on hackmd.io