HTML is stripped from runkit source

[joelnet]

Describe the bug
HTML is incorrectly stripped from runkit code blocks.

To Reproduce

1. Click WRITE A POST.
2. Paste into body:

{% runkit %}
const { ValueViewerSymbol } = require("@runkit/value-viewer");

const myCustomObject = {
    [ValueViewerSymbol]: {
        title: "My First Viewer",
        HTML: "<marquee>🍔Hello, World!🍔</marquee>"
    }
};
{% endrunkit %} 

3. Click SAVE POST
4. See code in runkit (incorrectly) translated into:

const { ValueViewerSymbol } = require("@runkit/value-viewer");

const myCustomObject = {
    [ValueViewerSymbol]: {
        title: "My First Viewer",
        HTML: "🍔Hello, World!🍔"
    }
};

Expected behavior
Code was expected to keep <marquee> tags.

Additional Info
I can trick the parser by encoding my HTML.

This works:

{% runkit %}
const { ValueViewerSymbol } = require("@runkit/value-viewer");
const atob = require('atob-lite')

const myCustomObject = {
    [ValueViewerSymbol]: {
        title: "My First Viewer",
        HTML: atob('PG1hcnF1ZWU+SGVsbG8sIFdvcmxkITwvbWFycXVlZT4=')
    }
};
{% endrunkit %}

Strangely enough, this does not work:

{% runkit %}
const { ValueViewerSymbol } = require("@runkit/value-viewer");

const myCustomObject = {
    [ValueViewerSymbol]: {
        title: "My First Viewer",
        HTML: ["<", "m", "a", "r", "q", "u", "e", "e", ">", "🍔", "H", "e", "l", "l", "o", ",", " ", "W", "o", "r", "l", "d", "!", "🍔", "<", "/", "m", "a", "r", "q", "u", "e", "e", ">"].join('')
    }
};
{% endrunkit %}

as it (incorrectly) results in this:

const { ValueViewerSymbol } = require("@runkit/value-viewer");

const myCustomObject = {
    [ValueViewerSymbol]: {
        title: "My First Viewer",
        HTML: ["", "🍔", "H", "e", "l", "l", "o", ",", " ", "W", "o", "r", "l", "d", "!", "🍔", ""].join('')
    }
};

[Zhao-Andy]

Hey @joelnet, thanks for raising this. The Runkit tag's implementation could be reworked to solve this, but we'll have to be wary of security concerns.

[stale]

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue in 7 days. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If this issue still requires attention, please respond with a comment. Happy Coding!

[mariocsee]

markdown_parser.rb cleans up some HTML before liquid tags are processed. I have a method in #2160 that saves HTML within the CodePen Prefill Liquid Tag from being erased.

When that PR gets reviewed and possibly merged, I can extend that method to include runkit tag as well, which should prevent it from scrubbing <marquee> in your example.

[mzaini30]

I think, you have to write <marquee>

[Oghenebrume50]

Yes from what I can see this is how Runkit works probably because of security issues, @Zhao-Andy please what are the security concerns, and I would like to work on this, I would appreciate pointers on the best way to solve this issue

[cmgorton]

@forem-team is this an issue we would still like contributors to work on? If so could we add more context here for how they should begin working on this so we can highlight it to the community?

[csaltos]

The security implication is the following:

If the HTM is not cleaned then a <script> tag can be introduced ... and with that, an attacker can hijack the session of a potential victim or use Javascript to create content without authorization, etc.

IMPORTANT: sadly, as mentioned by @mzaini30 if you change the "<" character for < and ">" for > then the tag will go through (including script tags).

Here a sample of a possible script tag attack vector that is now open ->

{% runkit %}
const { ValueViewerSymbol } = require("@runkit/value-viewer");

const myCustomObject = {
    [ValueViewerSymbol]: {
        title: "My First Viewer",
        HTML: "<script>alert(\\"🍔Hello, World!🍔\\")</script>"
    }
};
{% endrunkit %} 

Use it with a new post and when run, the Javascript inside the HTML will run too (not good !!).

CONCLUSION: this ticket should not only fix the cleaning of an HTML for not to be too aggressive (for real valid cases like the present marquee example) but also it should be sanitized for preventing script injection (potential attack cases) ... WHY? -> because the "trick" of removing the "<" or ">" characters to prevent attacks is NOT working and we need a real sanitizer here.

[pawelborkar]

Hi @cmgorton,
I would like to work on this issue :-)

[cmgorton]

Hey @pawelborkar I just assigned you to Issue #11500 . Did you still want to work on this one as well. If so I will assign it to you. :)