Svelte: Bug: "<!-- <script></script> -->" does not work in JavaScript string

[xmeta]

Version

3.5.1

Reproduction Link

https://svelte.dev/repl/0523d3f524974a209df7c43aa087d50d?version=3.5.1

Steps to reproduce

1. Put '< !-- -->' in JavaScript string

alert("<!-- <script>console.log('Hello')</script> -->");

What is expected?

No error
https://jsfiddle.net/jo70b953/

What is actually happening?

Unterminated string constant (2:8)

[Panya]

Well, you can just escape the / in the closing script tag. See repl.

[xmeta]

@Panya Yes, I knew how to avoid this problem

this is bug report

[trbrc]

This is still not a bug.

You can't use the character sequence </script> anywhere inside a script tag, since it closes the script tag.

[Panya]

@xmeta not sure that Svelte' html parser will ever support all idiosyncrasies of real html parsers. I'd say it's waste of time.

[xmeta]

@trbrc You can run this code on your browser.

<script>
alert("<!-- <script>console.log('Hello')</script> -->");
</script>

but svelte can't.
Why this is not a bug?

[xmeta]

@Panya hmmm, Is svelte's html parser broken?

[Panya]

@xmeta the same way as Vue's one. It just uses more simplified rules for parsing content. I just say I think that supporting all html quirks is waste of time.

[xmeta]

Svelte does not support formal HTML?
I think it is necessary to add the specification to README.

@Panya I knew Vue is also buggy
https://github.com/vuejs/vue/issues/7989#issue-312468149

[trbrc]

@xmeta You're right, that specific pattern is supported in HTML, although as it doesn't seem particularly unusual that it causes tools to break (see e.g. the syntax highlighting in your JS Bin). There are plenty of other things that you can do in HTML that you can't do in Svelte, for example have an arbitrary number of script tags and style tags, or use <!-- as a JavaScript comment.

[Panya]

@xmeta

Svelte does not support formal HTML?

Yes, e.g. you can't write like this:

<p>foo
<p>bar

or use multiple script tags (except context="module"), or use external script tags, ...

[xmeta]

@Panya @trbrc
Ok, but Svelte does not support formal JavaScript too.

like this:
alert("<!-- <script></script> -->");
Does Svelte have plans to support formal JavaScript?

[Panya]

@xmeta what's your real use case for this? It's supported in an external script if you really want to use this kind of messy stuff. See repl.

[xmeta]

@Panya I knew how to avoid this problem
this is bug report

[Panya]

@xmeta

what's your real use case for this?

[xmeta]

@Panya
This bug report is not for me to use, but to prevent someone from using this bug.
Sloppy work creates a security hole.

Unexpected behavior is a vulnerability.

[Panya]

@xmeta how some behaviour that potentially prevents XSS could create a security hole?
Without context and PoC it's just meaningless words.

You asked:

Does Svelte have plans to support formal JavaScript?

I provided an example that it's not true (Svelte already supports formal JS).

[pngwn]

This is not a bug. You've already had an explanation for this behaviour in another issue. Some environments handle all kinds of strange quirks, there is plenty of invalid html and javascript that browsers support. That doesn't mean svelte will support them.

The spec simply looks for a closing script tag in order to close the script block, the HTML parser has no concept of javascript (in fact script tags can be used for any language not just javascript) so it doesn't matter if the </script> is in a comment or in a string, it should still close the block. It gets more complicated if that string contains an HTML comment which contains a script tag, as in your example, but that isn't an edge-case worth supporting in Svelte.

If you want to do this, escape it.

[Rich-Harris]

I hadn't seen the commented example before. I think that is a bug that should probably get fixed, albeit a low priority one. We would welcome pull requests.

@Panya you can use unclosed <p> tags FYI — Svelte only asks that your top-level elements be closed, to eliminate ambiguity 😀

@xmeta Please don't open multiple issues for the same thing — it's spammy and annoying. If an issue is closed in error, we can reopen it

[xmeta]

@Panya
I didn't talk about only XSS. There're a lot of possibilities.

@pngwn

If you want to do this, escape it.

This bug report is not for me to use, but to prevent someone from using this bug.
Sloppy work creates a security hole.

Unexpected behavior is a vulnerability.

@Rich-Harris
I did not think these issues are about the same thing. sorry

[Panya]

@xmeta

I didn't talk about only XSS. There're a lot of possibilities.

Could you provide an example of such possibility related to this issue?

[Ryuno-Ki]

Hm, I agree, that using a fuzzer might be a good idea to uncover edge cases and thus prevent security vulnerabilites.

However, it might help more if you could link to some issues (e.g. in other libraries), where a missing safeguard led to a security exploit.

[Conduitry]

Closing this, as #3047 seems to convey the actual underlying issue, and also since there was a lot of weird drama here. Whether #3047 will ever be fixed, I cannot say.