Collection of potential security issues in Jellyfin

[GermanCoding]

Collection of potential security issues in Jellyfin

This is a non exhaustive list of potential security issues found in Jellyfin. Some of these might cause controversy. Some of these are design flaws and require breaking changes to correct them. I tried to categorize the issues in general topics. Expansions of this list are welcome. Most of the items on this list are not meant for immediate fixing, but rather for long term planning with a 11.0+ future release in mind, where significant (breaking) changes are allowed. My intention is not to highlight why these issues are here, but what could be done to improve the situation.

Unauthenticated endpoints

• WebSocket connection doesn't require authentication #5689 WebSocket connection doesn't require authentication
• Video streams completely unauthenticated #1501 Video streams completely unauthenticated (VideosController, HlsSegmentController, LiveTVController)
  • Summary: The current implementation allows direct play without authentication. Transcoded media is partially authenticated (old HLS only has metadata protection, new HLS protects all). The Jellyfin web client does provide an api_key parameter in the URL for all some stream types¹, but it isn't checked.
  • HlsSegmentController has a comment saying "Can't require authentication just yet due to seeing some requests come from Chrome without full query string"
  • This also applied to streams in LiveTvController endpoints. Those are also not authenticated. Update: Appears to be resolved in 10.8.9
  • Potential fix: Require authentication on all video streams, both direct streams as well as any transcoded media.
• "Gets subtitles in a specified format" and "Gets subtitles in a specified format" (SubtitleController) endpoints are unauthenticated
  • Allows anyone who is able to retrieve the item id to download subtitles, even if not logged in
  • Potential fix: Authenticate both endpoints
• All endpoints in AudioController are unauthenticated
  • Has a comment saying "TODO: In order to authenticate this in the future, Dlna playback will require updating"
  • Currently affects "GetAudioStream()" and "GetAudioStreamByContainer()" endpoints. Anyone with knowledge about item ID's can retrieve audio from the server, even if not logged in.
  • Potential fix: Authenticate endpoints
• All Most endpoints in DashboardController are unauthenticated [Update: Partially authenticated since 10.8b1]
  • As of 10.7, this controller apparently contains admin dashboard (plugin) configuration pages. While the pages alone are not highly sensitive by itself (the controller only returns the list & html pages without config data), the endpoints being completly unauthenticated allows anyone to retrieve information about the server, e.g this leaks the installed plugins.
  • Potential fix: Require admin privileges (at least user privileges) to access these pages
• All (raw) image endpoints in ImageByNameController, ImageController & RemoteImageController are unauthenticated
  • This allows probing on whether a specific image exists on the server by guessing item id's (which can be done without too much trouble, as item ids are based on filepath and filename information) and then checking on what content (movies, series etc) exist on a given server, without having an account.
  • Potential fix: This is tricky, because the images are loaded by the browser itself, so currently no authentication is provided. Below I propose a new authentication mechanism which would also fix this issue, allowing these endpoints to be authenticated.
• PluginsController only requires user privileges for potentially sensitive actions
  • Includes, but is not limited to: Listing all plugins on the server without being admin, changing plugin settings, listing plugin settings without being admin. This includes the possibility of retrieving LDAP access credentials without admin privileges. In specific conditions it can also lead to attacks that leak entire user passwords, e.g by changing the remote LDAP server to an attacker controlled server.
  • Potential fix: Require admin privileges for all potentially sensitive plugin endpoints, e.g all non-legacy endpoints.
• GetUsers() endpoint (UserController) only requires user privileges
  • Allows listing of any user, id included, on the server, not limited to public users. Can leverage massively more invasive attacks due to further issues with user id authentication, see below.
  • Potential fix: Require admin privileges on this endpoint
• GetUser() endpoint (UserController) is fully unauthenticated
  • Allows an attacker with knowledge of user id's (easily retrievable for authenticated users due to open /Users endpoint, see above) to get the entire user configuration for a given user, potentially containing sensitive or private information. Examples include the authentication provider, last login dates as well as bruteforce login information (login attempt counters).
  • Potential fix: Require user privileges on this endpoint. Better would be to only allow users to retrieve their own data, only admins should have the ability to see all users. See below for issues relating to per-user authentication.
• GetUserViews() & GetGroupingOptions() (UserViewsController) endpoints are unauthenticated [Update: Resolved in 10.8b1]
  • Allows anyone with knowledge of a user id to retrieve potentially user-personalized data of a user, without being authenticated as this user (also works without having an account on the server). Potentially leaks personal data of users.
  • Potential fix: Require user authentication (ideally specific per-user authentication) for these endpoints.
• GetAttachment() (VideoAttachmentsController) endpoint is unauthenticated
  • Allows anyone with knowdlege of a video id to retrieve any attachments, without requiring valid credentials
  • Potential fix: Require authentication on this endpoint
• GetSystemInfo() (SystemController) is unauthenticated
  • Leaks internal information about the server
  • Potential fix: Only allow during first time setup, then require admin access
• Branding/Splashscreen API endpoint (ImageController) is unauthenticated [New in 10.8]
  • Returns an image auto-generated from available media on the server. As this image is publicy available, this partially leaks metadata about media available on the server.
  • Potential fix: Require authentication on this endpoint (may be difficult without auth rewrite, see other image issues above)

¹ except HlsSegmentController because the m3u playlist generated doesn't contain parameters

Authentication and user ids are decoupled

This is basically a single issue, however because it affects at least 27 controllers (+ many more endpoints) I'm listing this an issue category. This issue is related to #5210. Quoting my statements from that issue:

As soon as you know a user's id, you do really get some data. If you modify your local storage to use an admin's user id, the UI starts showing the admin dashboard and the admin user's data (continue watching, favorites etc), but you cannot perform any admin-only API calls there as your session is still not privileged.

You can still mess with the others user data - you can for example list or modify their favorites, recently watched, their progress on movies and series, see what they watched and so on, but you do not gain admin access. You basically get access to other users data, without gaining their privileges.

It's a bit of a weakness of the Jellyfin API, that simply having a user's id + having some authentication token (from any other user) exposes much of the data of the other user. Most of the API simply takes the user id as a parameter, without checking if the current session is allowed to view data of that user.

Explanation: Because the API isn't checking whether the session authorization matches the id of the requested user, any authenticated user can edit every other user. This includes, but is not limited to, viewing & modifying favorites, watch progress, customizations, profile settings (except passwords), views and many more. The API only performs this check when attempting to perform actions such as changing a password: Then a function "assertCanUpdateUser()" from RequestHelpers is called to ensure that the session auth is valid to modify the password. Such a check, which binds auth information to the user id, should be present (almost) everywhere to ensure that users do not modify other users data. Exceptions could be made to admin users, but normal users should not be able to view or edit other users.

Update: Since 10.9, basic checking of update permissions is now performed on (most?) endpoints that update user data. It is still possible to read another user's data without admin privileges.

The only requirement for this is knowledge of a user id, which is currently trivially easy due to loose permissions on the /Users endpoint.

Issues related to how authentication is handled

Authentication information in HTTP URL's - the api_key parameter

Related: #3492

The jellyfin API currently accepts authentication either via a custom header, X-Emby-Authorization (or similar) or via a get parameter, api_key, which is used sometimes by the web client. The latter is problematic because having secrets in a URL is generally not a great idea. A user who copies a URL maybe isn't aware that the URL contains secrets. Especially stream URLs are very long, because they contain many parameters and so the api_key parameter is easily overlooked. A user who carelessly shares such URL exposes their credentials to the world.

In addition, server admins who run jellyfin behind some type of (reverse) proxy almost always make some type of request logging. Request logging almost always includes the entire request url. This exposes the secret parameter to a logfile, making the logfile itself a precious secret. This is dangerous, as even too wide permissions on a single logfile can expose session credentials. On most webservers it is possible to censor such parameters before writing to a logfile, but this isn't currently explained or mentioned anywhere in the Jellyfin docs. (I do plan to make such a PR later to the docs page, if I don't forget it). Update: The PR is now live.

This issue - secrets in URLs - is long known to web developers. Sometimes it can't be avoided, but I believe the best way going forward would be to get rid of api_key parameters in URLs. It prevents many dragons lurking around.

Authentication information is stored in browsers local storage

Another rather big topic, with potential for controversy. Currently client authentication is handled by Javascript reading secrets from the browsers local storage and sending this via a custom HTTP header (or sometimes crafting URLs with an api_key parameter).

This is bad design, because a single XSS issue is enough to read out the local storage, extracting user credentials (session auth tokens), thus hijacking a user's entire session with a Jellyfin server.

Many people don't like Cookies, I know, but until we have something better (like HTTP state tokens or so) Cookies are the best way to handle this going forward. Authentication data should be stored in a HTTP Cookie with Secure, HttpOnly and SameSite=Lax/Strict attributes set. A HttpOnly cookie prevents JavaScript from accessing user credentials, even if an attacker has JavaScript access due to an XSS issue. Right now, Jellyfin has some CSRF protection because on cross site requests no authentication data is send. SameSite attributes produce similar effects, hopefully preventing CSRF issues.

This would also mean that the browser now includes authentication information on every request, thus making changes to authentication (see "Unauthenticated endpoints" above) much easier.

[joshuaboniface]

Thank you for this list. We are aware of quite a few, but for reasons of backwards compatibility they've never been fixed. We'd definitely like to but doing so in a non-disruptive way is the hard part.

[the-lazy-fox]

@GermanCoding Huge work and audit you did !!!
@joshuaboniface honestly, I think that the security of the worldwide community is much more important than backwards compatibility, don’t you?
Is there a way to put that on top of the pile?

[anthonylavado]

@the-lazy-fox The importance is understood, we just have to be very careful now that we have (an estimated) 35k+ installs, which is probably on the low side.

As @GermanCoding explains, some of this involves breaking changes, which will require everything to update, and will need extensive changes on the DLNA side. We did some enhancement for some endpoints with 10.7, but obviously more are planned.

There's probably about a dozen third party clients to notify when we do make changes...

[joshuaboniface]

@the-lazy-fox Security is indeed important, and I don't want to give the impression that we don't care about security. The issue is that a lot of breaking changes, especially if they affect well-used functionality, is not a good route to take in the name of security. We must have migration paths in place and carefully consider the impact these changes have on various clients, both internal and external, as well as on existing behaviour that users depend on.

Ultimately we expect users to have some ideas of the risks they take exposing instances to the Internet. No software is totally secure, and we're nowhere near the top - we admit that freely. A lot of this stuff is inherited from Emby's poor coding and security practices, and while we would like to rewrite everything to fix it all, volunteer time is limited. We're happy to take any and all PRs to fix security issues, as long as they adhere to the principle that functionality must be preserved and the changes must be transparent to a user with no knowledge of the details.

[seniorm0ment]

Maybe make some of the stuff that can cause breakage toggleable, so users can choose if they want a "hardened install" based on features they may not ever use anyways.
As long as there are no chances of a setting just corrupting the db or deleting all the users files ofc.

But yes, I think security is extremely important and should be taken very seriously. Breakage is an issue as well however, but it depends on the type of breakage. Hardening always has trade offs, thats why do what you can, offer the stuff that would inevitably break stuff as toggleable options that could explain what can break.

Any route, I definitely hope to see a strong stance taken on security with the future of Jellyfin, and thank the devs for all their time and efforts.

[CatSamaDD]

mark.
hope the security problem can be fixed