BuilderIO / partytown

Relocate resource intensive third-party scripts off of the main thread and into a web worker. 🎉
https://partytown.builder.io
MIT License
13.06k stars 434 forks source link

resolveUrl security concerns #53

Closed ntkoopman closed 2 years ago

ntkoopman commented 2 years ago

Currently, it looks like the recommendation for solving CORS issues is to proxy those requests from the same domain. I'm a bit hesitant to do this, because it would be a potential security concern (a malicious user could run their script from our domain).

I would feel a lot better if the script was encoded in such a way that is wasn't a valid JS file, but that would require to post process the file. Another thing that would be great is to have the importing script url in the resolve method so CORS could be reimplemented on the proxy.

adamdbradley commented 2 years ago

I think you're correct and tried to reflect it in the docs: https://github.com/BuilderIO/partytown/wiki/Proxying-Requests

Do you have a recommendation on how to best describe this?

cihadhoruzoglu commented 2 years ago

Maybe you can add extra domain check step in your server like whitelist domains array

adamdbradley commented 2 years ago

The docs recommend to use another domain.

ntkoopman commented 2 years ago

Sorry for not replying earlier. I think the updated docs explain it well enough. It's also possible to use a dummy content type if you really don't want anybody executing it as a script.

linuxd3v commented 2 years ago

I think the updated docs explain it well enough. It's also possible to use a dummy content type if you really don't want anybody executing it as a script.

even with using different domain - this reverse proxy is essentially "open proxy" - is it not? the way I read it - anyone could proxy any url through it - unless there is a detail i'm missing?

ntkoopman commented 2 years ago

I don't think you are missing anything. You can limit the scope by only allowing *.js, or use an explicit allowlist and only proxy dependencies that actually need this workaround.