Meta refresh redirect to any url that contains "fp" after www.adobe.com incorrectly shows Flash content permission dialog
Reported by
mihai.ba...@gmail.com,
Apr 7 2017
|
|||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36 Example URL: Steps to reproduce the problem: 1. Open an HTML file that does a <meta http-equiv="refresh"> to a particular any URL that contains "www.adobe.com/go/" and "fp" (see flash-content-permission-bug-reduced.html attached) 2. On fresh Chrome install (or if no prior consent was given), the user is prompted to allow execution of Flash content even when there is no Flash content on the target page (see pop-up-screenshot.png attached) What is the expected behavior? User is not prompted to allow Flash content when there's no Flash content on the target page What went wrong? User is shown Flash content permission pop-up Does it occur on multiple sites: Yes Is it a problem with a plugin? Yes Flash / Pepper Did this work before? N/A Does this work in other browsers? Yes Chrome version: 57.0.2987.133 Channel: stable OS Version: OS X 10.11.6 Flash Version: Shockwave Flash 25.0 r0 The bug was initially reported by users that were being prompted for Flash permissions while attempting to SSO on some adobe.com pages (see flash-content-permission-bug-initial.html for a (redacted) real-life sample of content triggering the bug). This bug breaks the login flow to Adobe.com properties, as some SSO mechanisms use HTML meta refresh redirects in order to achieve SSO. This bug breaks the flow from the user, whenever the token payload contains the "fp" - which is not something that can be fixed on our end since the user tokens passed through the URL contain base64-encoded binary payloads. The bug seems to be caused by an too lax check for a redirect to the download page for the Flash Player, as identified by the www.adobe.com/go/fp URL. However, www.adobe.com/go/ is a URL shortner/indexer used by multiple Adobe web sites, including for passing URL parameters to the target URL.
,
Apr 7 2017
,
Apr 7 2017
I found the offending line of code - it's seems to be caused by https://cs.chromium.org/chromium/src/chrome/browser/plugins/flash_download_interception.cc?rcl=539e731c557cbce418d3ac1ea315f6cc9bd0e21f&l=34 and, more specifically, by the `.com/go.*` part of the RegEx. It basically allows Chrome to consume the "fp" string arbitrarily deep in a URL, possibly way past the query / fragment part of the URL.
,
Apr 7 2017
Tommy, could you take a look?
,
Apr 7 2017
,
Apr 7 2017
,
Apr 10 2017
Patch is being reviewed here: https://codereview.chromium.org/2811903002/
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3a3243c913d98568a9b0edb4018ad3275ae2a8f0 commit 3a3243c913d98568a9b0edb4018ad3275ae2a8f0 Author: tommycli <tommycli@chromium.org> Date: Wed Apr 12 15:45:29 2017 [HBD] Tighten-up which adobe.com/go links are intercepted as Flash URLs BUG= 709292 Review-Url: https://codereview.chromium.org/2811903002 Cr-Commit-Position: refs/heads/master@{#464039} [modify] https://crrev.com/3a3243c913d98568a9b0edb4018ad3275ae2a8f0/chrome/browser/plugins/flash_download_interception.cc [modify] https://crrev.com/3a3243c913d98568a9b0edb4018ad3275ae2a8f0/chrome/browser/plugins/flash_download_interception_unittest.cc
,
Apr 12 2017
Should be resolved on tree now. I think this is also fairly easily mergable to 58.
,
Apr 12 2017
This bug requires manual review: We are only 12 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 12 2017
Hi - can you please confirm if this well tested in Canary, and has appropriate testing?
,
Apr 12 2017
+raymes It has good unit tests. We can let it bake onto Canary (tomorrow or day after) if you want -- I just wanted to request it ASAP since we're 12 days from stable. I think it's a pretty safe merge, but I'll defer to raymes (reviewer) for a second opinion.
,
Apr 12 2017
,
Apr 12 2017
Regexes are notably easy to mess up and I think the consequences are fairly significant (failing to navigate). Could we wait for M59?
,
Apr 12 2017
raymes: That's fine with me. I'll withdraw my merge req then.
,
Apr 12 2017
Sounds good, thanks! |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by mihai.ba...@gmail.com
, Apr 7 2017