auth0 / auth0-oidc-client-net

OIDC Client for .NET Desktop and Mobile applications
https://auth0.github.io/auth0-oidc-client-net/
Apache License 2.0
86 stars 49 forks source link

Potential security risk in Android, Xamarin.droid and browser based Auth0 by allowing going back and forth between login screen without logout/login in between #204

Closed Hiilbig closed 3 years ago

Hiilbig commented 3 years ago

(This is an issue that is based upon a chat about my suggested fix for the issue #202 of leaving the Auth-window open on Xamarin.droid):

What's the purpose of using NoHistory here ? As I understand correctly, ClearTop should be the one solving the issue, or is this incorrect?

_Originally posted by @frederikprijck in https://github.com/auth0/auth0-oidc-client-net/pull/203#discussion_r643216631_

Hiilbig commented 3 years ago

Capturing the conversation from the PR review on #203 :

if (AutoCloseBrowser) { customTabsIntent.Intent.AddFlags(ActivityFlags.NoHistory);
 
@frederikprijck 

frederikprijck 23 hours ago 
Member

What's the purpose of using NoHistory here ? As I understand correctly, ClearTop should be the one solving the issue, or is this incorrect?

 
@Hiilbig 

Hiilbig 22 hours ago 
Author

The reason I included the "NoHistory" flag was based on the description found here: https://developer.android.com/guide/topics/manifest/activity-element#nohist
I did not want the Auth0 window to be accessible with the "Back" command in Android. Even if it Auth0 activity had been closed, a new would be created, letting the user come back to the login screen.

The FLAG_ACTIVITY_CLEAR_TOP is used for clearing any other activities on the navigation stack and return navigation to the initial (old) activity and then let it receive the intent.

 
@frederikprijck 

frederikprijck 19 hours ago   
Member

I am curious to understand what the intention is for adding NoHistory here. How does it relate to the problem mentioned about the browser not being closed?

What is the actual issue/risk you see with the login page being part of the history?

 
@Hiilbig 

Hiilbig 19 hours ago 
Author

By leaving in the history the login page, the users could end up going "back" to the login page and login again, without a proper logout first.
There are some behavior questions that need to be answered for this, like

  • What happens with the app if the user does another login?
  • What happens with the app if the user login with a different account?
  • How do we manage the transition "Back" to the login page? Do we prompt the user? Do we auto-logout? What if the "back" was an accident?

The only way to avoid this easily, is to only be able to logout and then login again - and to make sure one user does not have access to the previous users information, is to clear the navigation history.

 
@frederikprijck 

frederikprijck 18 hours ago   
Member

All our browser-based SDKs (https://github.com/auth0/auth0-spa-js, https://github.com/auth0/auth0-angular, https://github.com/auth0/auth0-react) as well as our mobile Java SDK (https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/provider/AuthenticationActivity.kt#L101) end up having the login page end up in history, which is why I am not sure we should add it here.

The problems you describe span all of these SDKs and haven't shown to be an actual issue so far.

The intention here is solving an issue that seems to be unrelated to removing the login page from the browser history, if you believe this is an issue I would recommend opening an issue so we can track it separately and take this internally to discuss with all of the maintainers of the mentioned SDKs that are impacted by the login page ending up in the browser's history.

 
@Hiilbig 

Hiilbig 14 minutes ago 
Author

It might be an edge case that no-one else have given much thought. I do not know if this is an issue for Xamarin.droid only, but I do not think it is. I strongly believe there is a genuine security risk for being able to go back to the login screen and basically be able to go "forward" again, not knowing who is logged in.
If the decision is to leave out that part of it, I would strongly recommend a solution where the flag can be added or removed by choice.
I will create a separate issue for this.

jeremy-bridges commented 3 years ago

I am a co-worker of Thomas and interested in this fix too. Our main complaint here is that the login workflow needs a browser out of convenience. OAuth authentications work much better in this environment. However, the login interchange for all purposes is part of the app using the Auth0 library, not Chrome/Firefox/Edge. This is why it seems best to us to remove this page from the browser's history. It is unimportant to the user in the context of the browser app. It is only important to the user in the context of the Auth0-using app, during the login process. The only purpose it serves to leave the site in the browser's history is for a casual hacker. Without using proxy/man-in-the-middle attacks, they can gain the login endpoints for further exploits. Removing the login page from the history doesn't deter more serious attackers, but it does raise the bar without any consequences to functionality. Does that make sense?

frederikprijck commented 3 years ago

Thanks for raising this concern,

We had a conversation internally and believe potential vulnerabilities like this are supposed to be logged through https://auth0.com/whitehat, especially given they are not unique to our .NET OIDC SDK's.

My apologies for guiding you to create an issue while I should have probably sent you to that link in the first place.

Reporting it through https://auth0.com/whitehat will ensure it reaches the correct people to look into it from a broader point of view, before discussing it on a per SDK basis.

Closing this, as it will be tracked through a report on https://auth0.com/whitehat and escalated internally when appropriate.