okta / okta-auth-js

The official js wrapper around Okta's auth API
Other
453 stars 265 forks source link

Default restoreOriginalUri to redirect securely #913

Open rwev opened 3 years ago

rwev commented 3 years ago

The current defaults around restoreOriginalUri make applications vulnerable to insecure redirects. An attacker has the ability to send the user to an arbitrary page if a originalUri is being set dynamically.

If no restoreOriginalUri callback is provided in configuration, handleLoginRedirect uses window.location.replace to redirect back to the originalUri:

This default makes the application vulnerable to open redirect attacks if the originalUri is being set dynamically, e.g.


const someDynamicOriginalUri = 'https://somemalicioushost.com'
this.oktaAuthService.setOriginalUri(someDynamicOriginalUri)
...
this.oktaAuthService.handleLoginRedirect(tokens)

will indeed take the user to the malicious host.

The default restoreOriginalUri behavior should sanitize the originalUri to at least preserve the trusted host/origin, and therefore mitigate this exposure to insecure redirects.

The code to do this already appears to exist in the repository as part of the test configuration with toRelativeUrl.

I propose using toRelativeUrl in handleLoginRedirect in the default else block instead of blindly replacing the location:

// Redirect to originalUri
const { restoreOriginalUri } = this.options;
if (restoreOriginalUri) {
  await restoreOriginalUri(this, originalUri);
} else {
  // window.location.replace(originalUri);
  window.location.replace(toRelativeUrl(originalUri, window.location.origin));
}

CWE-601: URL Redirection to Untrusted Site ('Open Redirect')

aarongranick-okta commented 3 years ago

@rwev Thank you for this great suggestion! Obviously, malicious code which has gained access to your oktaAuthService instance could do many, many bad things. For a secure web application, this should never happen, but it does make sense to limit the potential damage in this scenario. I've created an internal issue, OKTA-423441, so that we can prioritize this work.

rwev commented 3 years ago

This also applies to usages of window.location.assign in OktaAuth.signOut

    const logoutUri = this.getSignOutRedirectUrl({ ...options, postLogoutRedirectUri });
    // No logoutUri? This can happen if the storage was cleared.
    // Fallback to XHR signOut, then simulate a redirect to the post logout uri
    if (!logoutUri) {
      return this.closeSession() // can throw if the user cannot be signed out
      .then(function() {
        if (postLogoutRedirectUri === currentUri) {
          window.location.reload(); // force a hard reload if URI is not changing
        } else {
          window.location.assign(postLogoutRedirectUri);
        }
      });
    } else {
      // Flow ends with logout redirect
      window.location.assign(logoutUri);
    }
KatieKoslosky commented 2 years ago

Any updates on this? it did get flagged as a security vulnerability by veracode in our last scan