jellyfin / jellyfin

The Free Software Media System
https://jellyfin.org
GNU General Public License v2.0
33.5k stars 3.05k forks source link

Some security flaws in Jellyfin #1885

Closed teacupx closed 11 months ago

teacupx commented 4 years ago

Hello everyone. I noticed that running a scan towards some public Jellyfin server from https://observatory.mozilla.org reveals a few security flaws. More specifically:

I'm not an HTML security expert, but according to Mozilla these are not too difficult to correct (the webpage linked above gives information about how to do it). Could it be done in future versions of Jellyfin?

iotku commented 4 years ago

Might be worth modifying the documentation for reverse proxies with notes about general security headers.

They're not critically important in most cases, but they're not terrible practice either.

The CSP is the most "difficult" part to deal with as you somewhat have to work out a lot of settings and the rabbit hole goes pretty deep if you want a "perfectly" restricted CSP without breaking functionality and I believe as it stands jellyfin-web does a fair bit of "unsafe" inlines or possibly evals.

For example, my nginx config I have the following in my server directive (updated 10/19/19)

add_header X-Frame-Options "SAMEORIGIN"; add_header X-XSS-Protection "1; mode=block"; add_header X-Content-Type-Options "nosniff"; add_header Content-Security-Policy "default-src https: data: blob:; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' https://www.gstatic.com/cv/js/sender/v1/cast_sender.js; worker-src 'self' blob:; connect-src 'self'; object-src 'none'; frame-ancestors 'self'";

My CSP is pretty lax and could be restricted more for an improved "score", but at a glance it seems functional with jellyfin, a lot more effort would probably have to go in if you have multiple applications on the same subdomain.

teacupx commented 4 years ago

Might be worth modifying the documentation for reverse proxies with notes about general security headers.

Definitely something very useful for those of us who don't know where to start, and very easy to do for someone who has the knowledge.

@iotku Thank you very much for sharing your nginx reverse proxy configuration. Do you know, by chance, what would be the equivalent config for Apache?

JustAMan commented 4 years ago

@iotku would you mind making a PR to https://github.com/jellyfin/jellyfin-docs adding those directives?

This also seems to resonate with https://github.com/jellyfin/jellyfin/issues/879

iotku commented 4 years ago

As it stands currently a lot of the benefit of the CSP would be had if we didn't need script-src 'unsafe-inline'.

https://csp.withgoogle.com/docs/why-csp.html

Creating a policy which allows inline scripts (script-src 'unsafe-inline') or allows loading scripts from untrusted domains does not improve security because it doesn't protect the application from XSS. If an application cannot use a safe policy it will not benefit from CSP until all patterns incompatible with CSP are removed; as such, it is prudent to hold off on deploying a policy until the application can adopt strict CSP.

I believe most of that would be on the jellyfin-web side of things or perhaps the react client in the future.

dkanada commented 4 years ago

I'm not sure how much work it would be to remove all inline scripts except apploader.js which initializes everything. @iotku if you want to look into that we could integrate the CSP changes into master.

I have also seen issues with the current CSP when running the web client on its own using nginx, but hadn't looked into the required changes yet.

iotku commented 4 years ago

Most of the onclick elements I hit were on the dashboard (like the shutdown button) and a quick search of jellyfin-web looks like there's some on the setup wizard as well.

I've been fairly successful viewing media/searching etc without hitting any csp blocks outside of the dashboard, but I suspect there might be some I missed. I believe the reports plugin also utilizes inline scrips, though I haven't toyed with all the plugins and again that's on the dashboard.

Of particular usefulness is adding 'report-sample' to script-src while testing as it gives much more detail in the console about what triggered it

sevenrats commented 11 months ago

much of this has been addressed according to the linked issues. if parts of this issue still persist, please open new, more specific issues.