RegisterService Continue and Safe/Malicious External urls

On RegisterService line 137, assuming AutoLogin = true

Continue = request.Continue ?? base.Request.GetQueryStringOrForm(Keywords.ReturnUrl)

And AuthenticateService line 220 - 225

                var referrerUrl = request.Continue
                    ?? session.ReferrerUrl
                    ?? request.Continue
                    ?? base.Request.GetQueryStringOrForm(Keywords.ReturnUrl)
                    ?? this.Request.GetHeader(HttpHeaders.Referer)
                    ?? authProvider.CallbackUrl;

IMO it probably isn’t ok (security issue) to redirect on QueryString value (and probably HttpHeaders.Referer and I’m not sure about CallbackUrl, I didn’t follow the code thru the auth providers) without checking whether it’s a site/safe url. Bad actors could send them to register on the site and then redirect them elsewhere to a malicious site that looks like the victim site.

For example, the ReturnUrl could redirect them to a page on a malicious site that indicates their login is incorrect (when it was correct on the victim site) and have them sign in again and therefore collecting their information.

Example using SimpleAuth.Mvc:
This will sign you in and then redirect you to DuckDuckGo.com
https://mvc.netcore.io/Home/Login?redirect=https://duckduckgo.com&userName=test&password=test

Granted the attack vector on RegisterService would only work if request.Continue wasn’t filled.

Possible Solutions:

  1. a Continue validator (maybe default to same domain as request)
  2. a switch (true/false) to turn off querystring or Continue functionality
  3. whatever the genius called mythz comes up with :slight_smile:

Saw you (mythz) was typing as I updated with example.

Yeah I’m just trying to think about the security risks, it’s not going to be able to populate the sttic redirect links with any personal info. A lot of affiliates/referers have redirects to external sites. The only risk I can think of is that the user sees that the start of the URL is for your App, but clicking it redirects to another site.

Anwyay, I’ll provide a way to be able to validate the redirect links.

The main security risk is redirecting you after sign in to a malicious site and

  1. collecting your sign in information by asking you to sign in again
  2. if you signed in and the malicious website can make api/website calls to the victim site

I don’t see anywhere, SS would pass any info using redirect.

I think a Validator would work. Whatever default validator is created shouldn’t use Contains() as I’ve seen malicious sites use subdomains.

Victim website: MyWebsite.com
Malicious website: MyWebsite.com.HackMe.test
Note: I put tld .test so it doesn’t link to an actual site

Where a bad regex or a simple Contains() would match on the subdomain and pass the test. Maybe use Uri.Host .

I’m sure you have a versioning strategy, but I’d be fine with the minor release being where the Validator is null the use the existing logic otherwise use the Validator and then in the next major release change it.

External redirects are now disabled in latest v5.6.1 on MyGet by default, they can be re-enabled/overrided with:

new AuthFeature(...) {
    ValidateRedirectLink = AuthFeature.AllowAllRedirects 
}

going to rename it to the pluralized ValidateRedirectLinks version which reads better, tho it’s the default so you won’t need to reference it.

Here’s the default implementation of AuthFeature.NoExternalRedirects:

I.e. doesn’t allow any external redirects except for those starting with the BaseUrl.

I like it. :+1: Nice and clean. I also like how you opted for “secure by default” :smiley:

1 Like