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

Security issue with duplicit-load-prevention example #44

Closed
Baldinof opened this issue Mar 17, 2022 · 3 comments
Closed

Security issue with duplicit-load-prevention example #44

Baldinof opened this issue Mar 17, 2022 · 3 comments

Comments

@Baldinof
Copy link

Hi!

The duplicit-load-prevention example is subject to XSS injection.

The example add a /addTranslations route that seems to modify the translations stored in memory.

How to reproduce:

  • Start sveltekit in the example: npm run preview
  • call /addTranslations manually, with a malicious value for home.text:
curl 'http://localhost:3000/addTranslations' -X POST -d '{
    "translationProps":[
        {"en":
            {"menu.home":"home",
                "menu.about":"About",
                "menu.notification":"You have {{count:gt; 0:{{count}} new {{count; 1:message; default:messages}}!; default:no messages...}}",
                "home.title":"Welcome to SvelteKit",
                "home.text":"<script>alert(\"Malicious\")</script>Visit <a href=\"{{link}}\">kit.svelte.dev</a> to read the documentation"
            }
        },{"en":["menu",
                "home"]}]
}'

Results:

image

I don't really understand duplicit-load-prevention example but, considering the risk of using it, I think it should be removed from this repo (and maybe advertise that addTranslations() sould never be called with input from an untrusted source).

@jarda-svoboda
Copy link
Member

Hi @Baldinof thank you for letting me know!

Anyway, i don't think this is really about addTranslations - i would say it's more about Svelte's @html – as noted in Svelte's docs:

Svelte does not sanitize expressions before injecting HTML. If the data comes from an untrusted source, you must sanitize it, or you are exposing your users to an XSS vulnerability.

FYI the duplicit-load-prevention example is there, because currently SvelteKit does not provide a way how to load translations only once (on server) and pass it on to the client (during the first page load). It will be replaced later when layout endpoints are implemented...

Please, correct me if I'm wrong and I will reopen this issue...

@Baldinof
Copy link
Author

It's not just about XSS, even if properly escaped, the example still allows to change server translations :/

I don't think that's what users of this package would expect, by just reading the documentation, and take inspiration from the example you get a vulnerable app (without any warning in the example code).

If duplicit-load-prevention is really something needed, I think the example should show a secured way to do it.

@jarda-svoboda
Copy link
Member

Well, i don't have any stats, but I think that duplicit-load-prevention example is just an edge case for apps where every single api request matters. Anyway, what you are saying does make sense, so i'll add a note to the example's docs as this solution will be replaced before SvelteKit 1.0.0..

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