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

CDATA escaping for inline <script> and <style> tags #6

Open
kfigiela opened this issue Jun 21, 2024 · 2 comments
Open

CDATA escaping for inline <script> and <style> tags #6

kfigiela opened this issue Jun 21, 2024 · 2 comments

Comments

@kfigiela
Copy link
Contributor

#3 accidentally introduced a regression wrt. to CSS rendering. <script> and <style> tags are special in HTML, as they are encoded as CDATA. That essentially means – no escaping, but closing tag can't appear in the payload content and HTML does not provide a way to escape this.

At the moment:

  • <style> elements generated by web-view use regular text escaping, thus, if they contain any of escaped characters (<>&"' etc.) rendered CSS will be incorrect,
  • <script> element that we don't have a helper now will: either cause invalid JS to be rendered (if text is used) or invalid HTML (with the risk of opening XSS vulnerability) if raw is used – the latter was also the case for CSS before Escape HTML text and attributes #3 was merged.

For example, addClass $ cls "test" & prop "background" ("url('https://picsum.photos/200/300?random=1&foo=2=bar')" :: Text)) will generate incorrect result:

<style type='text/css'>
.empty { background:url(&#39;https://picsum.photos/200/300?random=1&amp;foo=2=bar&#39;) }
</style>

Potential solution could be to introduce External to AST, which would be always the same as Raw but would refuse to include any </ sequence which would prevent XSS/invalid HTML. This is what blaze-markup does for these tags.


Trivia: Historically, people would split string literals in JS payloads to "escape" them, e.g.

<script>document.write('<scr'+'ipt type="text/javascript" src="http://example.com/some.js"></sc'+'ript>');</script>
@seanhess
Copy link
Owner

That plans works for me!

@seanhess
Copy link
Owner

Let's make sure to add tests when we go in to fix

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

No branches or pull requests

2 participants