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

The emby/users/public API leaks TONS of potentially sensitive data #880

Closed
sparky8251 opened this issue Feb 13, 2019 · 14 comments · Fixed by #2492
Closed

The emby/users/public API leaks TONS of potentially sensitive data #880

sparky8251 opened this issue Feb 13, 2019 · 14 comments · Fixed by #2492
Labels
backend Related to the server backend bug Something isn't working confirmed This issue has been reviewed and confirmed security The issue is a security issue.

Comments

@sparky8251
Copy link
Contributor

Describe the bug
Poke the emby/users/public API and check the output. I've included a prettyfied output of what my server responds with below:

[
    {
        "Name":"sparky",
        "ServerId":"682ed54326bc4890a1ea096b249a0cd9",
        "Id":"e23fcdb75728411d94fe4e498ad00ea3",
        "PrimaryImageTag":"11773f4d0c3c1d11b9b655ea96d0db6f",
        "HasPassword":true,
        "HasConfiguredPassword":true,
        "HasConfiguredEasyPassword":false,
        "LastLoginDate":"2019-02-12T23:38:25.9829004Z",
        "LastActivityDate":"2019-02-13T01:50:53.6268151Z",
        "Configuration":
        {
            "AudioLanguagePreference":"",
            "PlayDefaultAudioTrack":true,
            "SubtitleLanguagePreference":"",
            "DisplayMissingEpisodes":false,
            "GroupedFolders":
            [
            "0c41907140d802bb58430fed7e2cd79e",
            "767bffe4f11c93ef34b805451a696a4e"
            ],
            "SubtitleMode":"Default",
            "DisplayCollectionsView":false,
            "EnableLocalPassword":false,
            "OrderedViews":
            [
            "cec55ee85f5e99eb9b3d743e43441280",
            "f137a2dd21bbc1b99aa5c0f6bf02a805"
            ],
            "LatestItemsExcludes":[],
            "MyMediaExcludes":[],
            "HidePlayedInLatest":true,
            "RememberAudioSelections":true,
            "RememberSubtitleSelections":true,
            "EnableNextEpisodeAutoPlay":true
        },
        "Policy":
        {
            "IsAdministrator":true,
            "IsHidden":false,
            "IsDisabled":false,
            "BlockedTags":[],
            "EnableUserPreferenceAccess":true,
            "AccessSchedules":[],
            "BlockUnratedItems":[],
            "EnableRemoteControlOfOtherUsers":true,
            "EnableSharedDeviceControl":true,
            "EnableRemoteAccess":true,
            "EnableLiveTvManagement":true,
            "EnableLiveTvAccess":true,
            "EnableMediaPlayback":true,
            "EnableAudioPlaybackTranscoding":true,
            "EnableVideoPlaybackTranscoding":true,
            "EnablePlaybackRemuxing":true,
            "EnableContentDeletion":true,
            "EnableContentDeletionFromFolders":[],
            "EnableContentDownloading":true,
            "EnableSyncTranscoding":true,
            "EnableMediaConversion":true,
            "EnabledDevices":[],
            "EnableAllDevices":false,
            "EnabledChannels":[],
            "EnableAllChannels":false,
            "EnabledFolders":[],
            "EnableAllFolders":true,
            "InvalidLoginAttemptCount":0,
            "EnablePublicSharing":true,
            "RemoteClientBitrateLimit":0,
            "AuthenticationProviderId":"Emby.Server.Implementations.Library.DefaultAuthenticationProvider"
        },
        "PrimaryImageAspectRatio":1
    }
]

To Reproduce

  1. curl serverip:serverport/emby/users/public
  2. View returned JSON

Expected behavior
I'm unsure where this API is used, but I know its used on the login page to populate the user selector. Expected behavior would be that this doesnt return whether or not the user is an admin, how many failed attempts at guessing a password are on record, if a password is set at all, etc etc.

Should only return the username and image path so the web UI can populate its user selector (and whatever else is critical for the client to function)

@sparky8251 sparky8251 added the bug Something isn't working label Feb 13, 2019
@sparky8251 sparky8251 changed the title The emby/users/public API leaks TONS of potentially sensitive data The emby/users/public API leaks TONS of potentially sensitive data Feb 13, 2019
@JustAMan JustAMan added the backend Related to the server backend label Feb 13, 2019
@h3ge
Copy link

h3ge commented Feb 15, 2019

/Users/Public only shows public users, if you want to disable this function, modify the code MediaBrowser.Api/UserService.cs #258

public object Get(GetPublicUsers request)
return new List<Controller.Entities.User>();

if you need the public function, you can override not needed fields with null values

@EraYaN
Copy link
Member

EraYaN commented Feb 16, 2019

Why would you need a public API anyway? For all people that host the server on the internet this is pretty bad. But then again I'm against all public APIs.

@cvium
Copy link
Member

cvium commented Feb 16, 2019

Why would you need a public API anyway? For all people that host the server on the internet this is pretty bad. But then again I'm against all public APIs.

How else would you get a list of users though

@EraYaN
Copy link
Member

EraYaN commented Feb 16, 2019

Users should be stored locally on whatever client is used, IMO enumerating users is no good for any system.

I get that it’s for a Windows like login screen, but that is not internet accessible.

@JustAMan
Copy link
Contributor

You can always hide the users to achieve what you want, but don't enforce this to all users.

@sparky8251
Copy link
Contributor Author

Well, even if we want it public its by far too public. It gives out a lot of information it shouldn't.

Shouldn't be told if a password is set when you select a user. It can automatically test a no password login in the background or something.

Also, why does it leak all of your settings, your rights, and even tell how many times you've failed to enter your password and how close you are to locking yourself out?

Lots of really dangerous stuff is just blasted out.

@EraYaN
Copy link
Member

EraYaN commented Feb 16, 2019

I would argue all "public" APIs should be behind an API key. The API key can be stored after the first user logs in (and thus get an authenticated user token, which we could use to "authenticate" the client to enumerate users and the like).

If you scan all endpoints the server in general spits out way too much, hence why you should never run this on the internet IMO. It's not ready.

@triDcontrols
Copy link

I cannot reproduce, I get a empty array [] as return when I try mine on 10.0.2 in docker container.

screen shot 2019-02-16 at 7 55 21 pm

@sparky8251
Copy link
Contributor Author

sparky8251 commented Feb 17, 2019

@triDcontrols do you have any users without the "hidden" option checked?

If you have all users set hidden, this API returns nothing. It only shows "public" users.

The above is a known work around for sealing this leak but it doesn't address the real problem.

@sparky8251
Copy link
Contributor Author

sparky8251 commented Feb 20, 2019

Well, doing some testing it appears there isn't much of the above that's required by the web UI at least.

To me, it seems the following keys are required for normal behavior:

Name
PrimaryImageTag
HasPassword

It seems that HasConfiguredPassword is also required, but only if you need to set a password after logging in. Really unsure if the value is obtained from the public API, and going by the code it looks like the check is only ran after you've successfully logged in. As in, not convinced that is a need to know bit for the public API.

Can find the only code reference to HasConfiguredPassword in this file. Unable to find any others as of right now. To me, looks like its moved from users to user and is now logged in. Likely obtaining this data from the AuthenticateByName response or one the secured Users responses.

@LogicalPhallacy
Copy link
Contributor

LogicalPhallacy commented Feb 20, 2019 via email

@sparky8251
Copy link
Contributor Author

sparky8251 commented Feb 20, 2019

Sent you a message via matrix to setup a time to test @LogicalPhallacy.

That said... Looking at these bits of code, its apparent this is a disaster.

There is this that defines the public users API route:

    [Route("/Users/Public", "GET", Summary = "Gets a list of publicly visible users for display on a login screen.")]
    public class GetPublicUsers : IReturn<UserDto[]>
    {
    }

Then there is this, which makes it apparent it uses what is presumably a secure API and filters out disabled and hidden users with 0 additional alterations:

        public object Get(GetPublicUsers request)
        {
            // If the startup wizard hasn't been completed then just return all users
            if (!_config.Configuration.IsStartupWizardCompleted)
            {
                return Get(new GetUsers
                {
                    IsDisabled = false
                });
            }

            return Get(new GetUsers
            {
                IsHidden = false,
                IsDisabled = false

            }, true, true);
        }

And finally, proof that it is in fact exposing a secure API endpoint through an insecure route:

    [Route("/Users", "GET", Summary = "Gets a list of users")]
    [Authenticated]
    public class GetUsers : IReturn<UserDto[]>
    {
        [ApiMember(Name = "IsHidden", Description = "Optional filter by IsHidden=true or false", IsRequired = false, DataType = "bool", ParameterType = "query", Verb = "GET")]
        public bool? IsHidden { get; set; }

        [ApiMember(Name = "IsDisabled", Description = "Optional filter by IsDisabled=true or false", IsRequired = false, DataType = "bool", ParameterType = "query", Verb = "GET")]
        public bool? IsDisabled { get; set; }

        [ApiMember(Name = "IsGuest", Description = "Optional filter by IsGuest=true or false", IsRequired = false, DataType = "bool", ParameterType = "query", Verb = "GET")]
        public bool? IsGuest { get; set; }
    }

Really explains why there is soo much that isn't needed by the client sent with every single response.

@stale
Copy link

stale bot commented Jul 29, 2019

Issues go stale after 60d of inactivity. Mark the issue as fresh by adding a comment or commit. Stale issues close after an additional 7d of inactivity. If this issue is safe to close now please do so. If you have any questions you can reach us on Matrix or Social Media.

@stale stale bot added the stale Stale and will be closed if no activity occurs label Jul 29, 2019
@stale stale bot closed this as completed Aug 5, 2019
Issue Triage for Main Repo automation moved this from Needs triage to Closed/Done Aug 5, 2019
@sparky8251 sparky8251 reopened this Sep 19, 2019
Issue Triage for Main Repo automation moved this from Closed/Done to Needs triage Sep 19, 2019
@stale stale bot removed the stale Stale and will be closed if no activity occurs label Sep 19, 2019
@stale
Copy link

stale bot commented Jan 17, 2020

This issue has gone 120 days without comment. To avoid abandoned issues, it will be closed in 21 days if there are no new comments.
If you're the original submitter of this issue, please comment confirming if this issue still affects you in the latest release or nightlies, or close the issue if it has been fixed. If you're another user also affected by this bug, please comment confirming so. Either action will remove the stale label.
This bot exists to prevent issues from becoming stale and forgotten. Jellyfin is always moving forward, and bugs are often fixed as side effects of other changes. We therefore ask that bug report authors remain vigilant about their issues to ensure they are closed if fixed, or re-confirmed - perhaps with fresh logs or reproduction examples - regularly. If you have any questions you can reach us on Matrix or Social Media.

@stale stale bot added the stale Stale and will be closed if no activity occurs label Jan 17, 2020
@JustAMan JustAMan added confirmed This issue has been reviewed and confirmed security The issue is a security issue. labels Jan 20, 2020
@stale stale bot removed the stale Stale and will be closed if no activity occurs label Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to the server backend bug Something isn't working confirmed This issue has been reviewed and confirmed security The issue is a security issue.
Projects
Development

Successfully merging a pull request may close this issue.

7 participants