XSS Vulnerability in Table Component due to HTML tags

[nas-tabchiche]

Hello,

I have been using the Skeleton component library at work and have come across a potential security vulnerability in the Table component. It parses raw HTML strings through HTML tags, which exposes it to cross-site scripting (XSS) attacks. I believe this needs to be addressed urgently.

Details:

When strings are passed to the table component, it gets rendered through HTML tags, therefore circumventing Svelte's string sanitation, making it prone to XSS.

skeleton/packages/skeleton/src/lib/components/Table/Table.svelte

Line 102 in 72bd820

{@html cell ? cell : '-'}

Why this is a serious security risk:

Here the main danger is the fact that developers using the Table component might not be aware that cells are rendered through HTML tags, which is very dangerous in the event that user-input strings are passed to the component without sanitation. Which is likely, as Svelte already sanitizes strings (outside of HTML tags).

XSS vulnerabilities open us up to a whole world of hurt:

• An attacker can manipulate the table's data or any other DOM elements.
• If an attacker can run arbitrary JavaScript code, they can access and send the user's cookies to a remote server, compromising their session.
• The attacker can create fake overlays or pop-ups asking the user for sensitive information.
• The attacker could use XSS to redirect the user to malicious websites that host malware.

Recommendations:

Best to worst in my opinion:

1. Opt-in HTML tags: Instead of default HTML parsing behavior, make it opt-in. This way, developers are actively making a choice to accept HTML content, hopefully with the understanding of its implications.
2. Opt-out HTML tags: If not opting in, allow developers to opt-out of the HTML parsing functionality. This would allow the patch to be non-breaking.
3. Warning in the documentation: At the very least, there should be a clear warning in the documentation stating that the table component parses raw HTML and that users should sanitize the input they pass to it.

Thank you for taking the time to consider this issue. I believe skeleton is bound to be a leader in the svelte space, and as such security should be paramount. As a user of the library, I am looking forward to seeing how it will evolve.

Best regards,

Nassim

[endigo9740]

@nas-tabchiche There's been a few instances over the project's history where folks have expected Skeleton to police and add guardrails to areas that are deemed potentially unsafe. The updates to the Modal/Dialog/Toast stores in v2 for example received a number of requests for this change - and have since received a number of complaints now that the change is implemented.

While I acknowledge that XSS vulnerabilities are to be taken seriously, I kind of teeter on whether or not we want Skeleton to be the authority on this. When it comes to XSS in Svelte specifically, SvelteKit provides protection with a simple configuration setting:
https://kit.svelte.dev/docs/configuration#csp

It's up to you, the user, to implement this. While we could add a warning about this solution on the Table Component specifically, it's not the only component using @html. Which means we then have to add yet another item onto a long list of existing prerequisites to ship both current and future components. Unfortunately, I think this may be out of scope for what Skeleton is and what we're trying to accomplish. We are NOT trying to teach you how to use SvelteKit and we're not trying to teach you proper security protocols. Regardless of the popularity of the library, I feel that is 100% out of scope.

Major contributors to Svelte seem to take a similar stance, that this is the responsibility of SvelteKit to provide a solution - which it does per the link above:
sveltejs/svelte#6423 (comment)

That said, I'll talk to the core team and determine if there's any action we wish to take. But for now, I'd suggest using the source shared above to handle this yourself.

[HugeLetters]

I agree options 1 or 2 should be added to the library - same goes for the Autocomplete component.
I think adding a prop to toggle this behavior should be a very easy-to-add solution.

While I agree sanitation should be handled by the library users - I don't think that's the case here.
As in when I render a button I(and I assume most people) usually do this:

<button>{label}</button>

not this

<button>{@html sanitize(label)}</button>

[atuttle]

I just want to chime in to say that I'm intentionally using HTML inside of table contents to accomplish functionality that isn't available in a Skeleton table otherwise. Whatever is done, if anything, I do hope that there is a way to opt-in to continuing to render raw html.

[endigo9740]

In an effort to prepare for Skeleton v3, we're consolidate some related issues down to a single ticket. This will ensure that we can see the full context of requests when the time comes to refactor and update this feature going forward. If you wish to add additional feedback or suggestions, please so here:

• NEXT: Internal code style guide #2361