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

Exception, when origin is null in CorsPolicyExtensions.cs #2318

Closed
aspnet-hello opened this issue Jan 1, 2018 · 7 comments
Closed

Exception, when origin is null in CorsPolicyExtensions.cs #2318

aspnet-hello opened this issue Jan 1, 2018 · 7 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed

Comments

@aspnet-hello
Copy link

aspnet-hello commented Jan 1, 2018

From glatzert on Wednesday, December 20, 2017 6:31:36 AM

After Login with ADFS, Chrome produces an POST request to my ASP.net Application, with the following Contents:

Metadata

Request URL:http://localhost:11645/signin-oidc
Request Method:POST
Status Code:500 Internal Server Error
Remote Address:[::1]:11645
Referrer Policy:no-referrer-when-downgrade

Request Headers

Provisional headers are shown
Content-Type:application/x-www-form-urlencoded
Origin:null
Upgrade-Insecure-Requests:1
User-Agent:Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.84 Safari/537.36

CorsPolicyExtension.IsOriginAnAllowedSubdomain will now throw the following:

System.UriFormatException: Invalid URI: The format of the URI could not be determined.

Which happens, because origin, is null and still gets passed to the check function.

Addendum:
Origin is not null as I triaged on my first try- it's "null" - which might or might not be a bug in Chrome ...

Copied from original issue: aspnet/CORS#137

@aspnet-hello
Copy link
Author

From mkArtakMSFT on Wednesday, December 27, 2017 9:57:32 AM

@jbagga, can you please investigate this?
@glatzert, can you please share a project we can repro with?

@aspnet-hello
Copy link
Author

From glatzert on Thursday, December 28, 2017 12:54:54 AM

@mkArtakMSFT I can make some project up, if you like, but essentially use

app.UseCors(cors => cors.SetIsOriginAllowedToAllowWildcardSubdomains()) anywhere, and then forge a POST with a header Origin: whatever-non-uir-you-like-here.

It boils down to missing this one in the code:

if (!Uri.IsWellFormedUriString(origin, UriKind.Absolute)) return true;

@aspnet-hello
Copy link
Author

From mkArtakMSFT on Thursday, December 28, 2017 9:54:43 AM

@glatzert, to avoid any misunderstandings, I would appreciate if you could still share the very-minimum project with a repro.

@aspnet-hello
Copy link
Author

From glatzert on Friday, December 29, 2017 12:46:09 AM

CORSRepro.zip
Here you go - Edge does not reproduce the Error, but Chrome will do. I did not test Firefox nor Safari

@jbagga
Copy link
Contributor

jbagga commented Jan 3, 2018

Thanks for letting us know. I am able to repro this. I will look into how to fix this.

@jbagga jbagga added 1 - Ready bug This issue describes a behavior which is not expected - a bug. and removed investigate 1 - Ready labels Jan 3, 2018
@jbagga
Copy link
Contributor

jbagga commented Jan 4, 2018

Please note I only investigated the CORS part of the repro (did not include auth yet).

This is where the exception gets thrown https://github.com/aspnet/CORS/blob/dev/src/Microsoft.AspNetCore.Cors/Infrastructure/CorsPolicyExtensions.cs#L19

@rynowak @kichalla Thoughts? I don't think it is safe to allow a malformed uri unless the policy states to allow any origin. According to w3c

If a cross-origin resource redirects to another resource at a new origin, the browser will set the value of the Origin header to null after redirecting. This prevents additional confused deputy attacks

It is not safe to assume that all malformed or null origin uri are in error

jbagga added a commit to aspnet/CORS that referenced this issue Jan 8, 2018
@jbagga jbagga added Done This issue has been fixed and removed 2 - Working labels Jan 8, 2018
@jbagga
Copy link
Contributor

jbagga commented Jan 8, 2018

Incorrectly formatted origin is treated as a malformed Origin header and not a valid allowed origin

@jbagga jbagga closed this as completed Jan 8, 2018
@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Nov 26, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

3 participants