Identityserver4: XSS in the RequestLoggerMiddleware.cs

[JameelNabbo]

Hi,
https://github.com/IdentityServer/IdentityServer4/blob/master/src/IdentityServer4/host/Extensions/RequestLoggerMiddleware.cs

in the LogForErrorContext method the PARAM httpContext is not filtred, and can be injected with XSS payload onerror="alert(String.fromCharCode(88,83,83)) in which can be triggred from the log as well.
static ILogger LogForErrorContext(HttpContext httpContext)
{
var request = httpContext.Request;

        var result = Log
            .ForContext("RequestHeaders", request.Headers.ToDictionary(h => h.Key, h => h.Value.ToString()), destructureObjects: true)
            .ForContext("RequestHost", request.Host)
            .ForContext("RequestProtocol", request.Protocol);

        if (request.HasFormContentType)
            result = result.ForContext("RequestForm", request.Form.ToDictionary(v => v.Key, v => v.Value.ToString()));

        return result;
    }

[leastprivilege]

XSS is very context specific. What you describe above would e.g. not do any harm in a console output (and we cannot know how you plan to output your log files).

IOW - you protect yourself against XSS with output encoding, not input sanitization.

As a side note - this logger is only part of our test host. Not IdentityServer.

[leastprivilege]

btw - are you the author of this

https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-12250

[scottbrady91]

Quick heads up, there are guidelines in place for reporting security issues: https://github.com/IdentityServer/IdentityServer4/blob/master/SECURITY.MD

[leastprivilege]

@JameelNabbo I contacted CVE to take the entry down, as it is obviously not a security problem. If you are the author please do the same.

For the "security researcher" I have advice for the future

• do better research
• learn about "responsible disclosure"

[pseudoramble]

This came up as an item for the team I'm on to check into since it shows up on CVE lists still. I wanted to share a bit more detailed explanation about some reasons why I believe this is not an issue for IdenttiyServer4 consumers.

(To be clear, I'm not a security researcher. But if the earlier points didn't clarify it enough, I think these points are more than sufficient to alleviate concerns for teams using IdentityServer).

• RequestLoggerMiddleware is marked internal to the EntityFramework portion of IdentityServer. That means another assembly can't reference to it unless it's part of the same assembly. A consumer cannot build this middleware into their own setup.
• RequestLoggerMiddleware is not set up behind the scenes in an app consuming IdentityServer. If you search for references to the middleware it only appears in the Startup.cs classes in the EntityFramework portion of IdentityServer. That's the code that runs the "test host" explained earlier in this thread. So only code used to test out the library makes use of this.
• Even if you were able to put a source code change into IdentityServer to make it publicly accessible, it will not be setup unless it's configured in a consumer's Startup.cs file while setting up other middleware.
• Even if you were able to change IdentityServer to make the middleware consumable and then inject it into a consumers application without them knowing about it and then submit JS to the instance of it, an application must consume these logs and interpret it as JS. At that point, the application reading those logs should take that input and be sanitizing them to be sure since at that point it's an input to that new application.

Hope this helps somebody else out.

[leastprivilege]

The real problem is that @JameelNabbo doesn’t know what he is talking about, ignored all best practices wrt to responsible disclosure and opened a CVE instead. Now he doesn’t even have the decency to correct this mistake. Case closed.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.