amphtml icon indicating copy to clipboard operation
amphtml copied to clipboard

amp-access: Return url redirect is not validated against Open Redirection Vulnerability

Open Joelsz opened this issue 5 years ago • 10 comments

What's the issue?

The default value of the "return" parameter for the amp-access login endpoint is set to the cdn.ampproject.org "login done" page with a "url" parameter that contains the original url of the AMP page, where the canonical backend is supposed to redirect after login or logout. Once redirected, the "login done" page is immediately redirecting back to the URL from the "url" parameter which is the original AMP page from where the authentication request originated from.

The issue is that the "url" parameter is not being validated by the domain, so when the "url" parameter is manually changed to another domain, it's still being redirected to that url and not being validated.

How do we reproduce the issue?

Let's say this is a valid login page on a site: https://www.canonical-site.com/login

The AMP default "return" parameter will be set as follows: https://www.canonical-site.com/login?return=https%3A%2F%2Fcdn.ampproject.org%2Fv0%2Famp-login-done-0.1.html%3Furl%3Dhttps%253A%252F%252Fwww.canonical-site.com%252Famp

The backend of https://www.canonical-site.com should verify the domain from the "return" parameter and block redirects to unknown sites, however, the https://www.canonical-site.com shouldn't need to verify the "url" parameter which is up to the "login done" page, and apparently it doesn't verify the "url" before redirecting.

So if a hacker changes the "url" parameter to some other domain it will still redirect: https://www.canonical-site.com/signout?return=https%3A%2F%2Fcdn.ampproject.org%2Fv0%2Famp-login-done-0.1.html%3Furl%3Dhttps%253A%252F%252Fwww.github.com

That essentially opens a back door for a Open Redirection Vulnerability on https://www.canonical-site.com when using the AMP login URL system.

This issue relates more to logout since in most cases there's no input required from the user to logout, so it will happen instantly and then redirect.

What browsers are affected?

All browsers

Which AMP version is affected?

Probably since amp-access "return" parameter was implemented.

Joelsz avatar Aug 11 '20 18:08 Joelsz

@ampproject/wg-access-subscriptions

dreamofabear avatar Aug 12 '20 15:08 dreamofabear

So if a hacker changes the "url" parameter to some other domain

How would this happen? An XSS on canonical-site.com?

dreamofabear avatar Aug 12 '20 16:08 dreamofabear

The hacker can share a modified url with someone who would recognize the domain as a legitimate site, but really it will redirect to the hacker's site which potentially can fake the user to input personal information by presenting similar content etc.

This example is a modified sharable link that can redirect anywhere, in this case to github.com: https://www.canonical-site.com/signout?return=https%3A%2F%2Fcdn.ampproject.org%2Fv0%2Famp-login-done-0.1.html%3Furl%3Dhttps%253A%252F%252Fwww.github.com

Joelsz avatar Aug 12 '20 18:08 Joelsz

Hmm the redirect is here https://github.com/ampproject/amphtml/blob/221939c0c7a0d42f66aa186c62a74ec2b3ebb0b6/extensions/amp-access/0.1/amp-login-done-dialog.js#L69

The question is what should we validate it against? It seems reasonable to assert the domain in the url param is either the referring domain or it's amp cdn equivalent. Are there any other valid use cases?

jpettitt avatar Aug 12 '20 19:08 jpettitt

On reflection that strategy won't work if the login is hosted on a different domain (eg 3p login system). I'm going to assign this to @dvoytenko since he wrote it.

jpettitt avatar Aug 12 '20 19:08 jpettitt

The redirect URL is currently, indeed, unbound. I believe in the past we considered this "ok" since the cdnl.ampproject.org is cookieless and no data is passed to the destination URL. It's indeed an open redirector, but I'm not quite seeing vulnerability here yet. Could you please clarify the vector here?

dvoytenko avatar Aug 18 '20 18:08 dvoytenko

From https://en.wikipedia.org/wiki/Phishing:

Phishing is the fraudulent attempt to obtain sensitive information or data, such as usernames, passwords and credit card details, by disguising oneself as a trustworthy entity in an electronic communication.[1][2] Typically carried out by email spoofing,[3] instant messaging,[4] and text messaging, phishing often directs users to enter personal information at a fake website which matches the look and feel of the legitimate site.

So the fraudster sends an email containing a link with the real - not a fake - canonical domain, thus giving even more confidence to the user that it's trustworthy, and they are able to then redirect to their own fake site and obtain information out of the user.

Joelsz avatar Aug 18 '20 22:08 Joelsz

Got it. Thanks for the explanation. We need to see how we can address this. Couple of things that come to mind:

  1. A simpler (though not complete) change would be to stop auto-redirecting and give user a link "Proceed to www.example.com".
  2. A more complete solution: proxy amp-login-done.html to the initiating CDN origin. E.g. https://example-com.cdn.ampproject.org/v0/amp-login-done.html and verify that the return URL's origin is equivalent to this one.

dvoytenko avatar Aug 19 '20 00:08 dvoytenko

To solution 1, we could check the referer first, and see if it matches the destination. We show the proceed to xxx message only if it does not match/is unknown.

powerivq avatar Oct 07 '20 19:10 powerivq

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 19 '25 08:07 stale[bot]