Modify

#13040 closed defect (fixed)

Script injection

Reported by: Jun Omae Owned by: tkob-trac
Priority: normal Component: MermaidMacro
Severity: normal Keywords:
Cc: Trac Release:

Description

Any scripts can be injected via id argument:

{{{#!Mermaid id="''><script>alert(42)</script>"
....
}}}
  • mermaidmacro/1.0/tracmermaid/mermaid.py

    diff --git a/mermaidmacro/1.0/tracmermaid/mermaid.py b/mermaidmacro/1.0/tracmermaid/mermaid.py
    index 0f871da..e50b617 100644
    a b class MermaidMacro(WikiMacroBase): 
    3333        if args == None or 'id' not in args:
    3434            id_attr = ''
    3535        else:
    36             id_attr = 'id=%s' % args['id']
     36            id_attr = 'id=%s' % escape(args['id'])
    3737        url_escaped_content = urllib2.quote(content)
    3838        div = """<div class="mermaid"
    3939                      %s

Attachments (2)

0005-TH13040-Fix-possible-script-injection-vulnerability.patch (1.1 KB) - added by Christian Boos 11 months ago.
incremental patch, on top of 004 in #13042
0005-TH13040-Fix-possible-script-injection-vulnerability.2.patch (1.3 KB) - added by Christian Boos 11 months ago.
take 2, add missing "" after id=

Download all attachments as: .zip

Change History (7)

comment:1 Changed 11 months ago by Jun Omae

The same issue can be occurred via wiki page named with "><script>alert(42)</script>.

  • mermaidmacro/1.0/tracmermaid/mermaid.py

    diff --git a/mermaidmacro/1.0/tracmermaid/mermaid.py b/mermaidmacro/1.0/tracmermaid/mermaid.py
    index 0f871da..c491e46 100644
    a b class MermaidMacro(WikiMacroBase): 
    4343                      data-mermaidsource="%s">%s</div>"""
    4444        return div % (
    4545                id_attr,
    46                 formatter.context.resource.realm,
    47                 formatter.context.resource.id,
    48                 formatter.context.resource.version or '',
     46                escape(formatter.context.resource.realm),
     47                escape(formatter.context.resource.id),
     48                escape(formatter.context.resource.version or ''),
    4949                escape(url_escaped_content),
    5050                escape(content))
    5151

comment:2 Changed 11 months ago by Christian Boos

Oh, good catch, didn't see that one...

Just in case you're also testing my changes, I even have one more pending improvement, the support for TracLinks in flow charts.

I'll first version the changes locally so that we can have separated patches per ticket.

Changed 11 months ago by Christian Boos

incremental patch, on top of 004 in #13042

comment:3 Changed 11 months ago by Christian Boos

For good measure. Now you have all my changes. Would be nice if tkob had a github repo for this plugin, but it seems not.

Changed 11 months ago by Christian Boos

take 2, add missing "" after id=

comment:4 Changed 11 months ago by Christian Boos

By the way, it seems possible to insert <script> tags via the "normal" mermaid editor. Try the following:

sq["node <b>text</b><script>alert('bad!')</script>"] --> ci((Circle shape))

in http://knsv.github.io/mermaid/live_editor/.

However, via the MermaidMacro it seems OK, the alert doesn't show up, though the text is still bold. I haven't understood yet the difference between the two situations, i.e. the generated markup seems to be the same. Maybe you have a clue and could tell if there's a possible injection risk via this method?

Last edited 11 months ago by Christian Boos (previous) (diff)

comment:5 Changed 11 months ago by tkob-trac

Resolution: fixed
Status: newclosed

In 16179:

Add escape to avoid script injection, fixes #13040

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain tkob-trac.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.