meacer@ -- based on some of the other dialog related spoofing bugs, assigning to you. please re-assign as appropriate or assign it back to me.

+CC: felt@

I can confirm the bug, although it doesn't always repro.

Avi: Does your plan to fix alert include suppressing dialogs coming from background tabs? 

My plan doesn't currently involve Androids. It is possible to bring them into it later, but right now it's desktop-only.

Okay, in that case we'll need to bring alerting tabs to foreground on Androids.

 Issue 677728  has been merged into this issue.

+xisigr who also reported this in  bug 677728 

Avi, what do you think is the right approach here? Should we suppress the dialog if it's in the background, or should we bring it to the foreground? I believe you had plans to do the former?

Ignore comment #11, you already answered it in comment #6 which means I fail at reading :)

NP.

Re comment 11, either is fine. Either suppress the dialog, or bring the page that spawned it forward. The choice is a design decision (which I'm slowly working on changing on the desktop) but if you don't keep the showing of dialogs in sync with the showing of pages that show them, you essentially get this URL spoof.

Thanks. It seems like suppressing background dialogs is the better solution (both for desktop parity and user experience) but might need an intent-to-deprecate, do you agree? I'm curious how much of a battle that will be versus simply bringing the tab into focus and fixing the immediate issue.

I have that on my list of things to eventually do, but alert() usage is high, and its use to pull webpages to the front is high. I'm collecting UMA stats to make the argument, but I suspect if you file the intent now it will not go well.

I would highly suggest going the other route :)

Sounds good, I'll leave fighting the good fight to you then :) I'll simply bring the tab back to focus.

luan.herrera, xisigr: Are you able to consistently reproduce this? The POCs don't work for me on a tip of tree build -- the tab in the background is already brought forward when it shows a dialog.

meacer:My POCs work well ,https://bugs.chromium.org/p/chromium/issues/detail?id=677728

Thanks, I tried on a couple of different devices and it looks like it reproduces when the tab strip is not visible (repros on phones but not tablets).

Similar to  bug 549724 , this also seems to be fixed, possibly by avi's change to suppress dialogs from swapped out frames ( bug 634108 ). I'm unable to reproduce it, the alert gets cancelled when the google.com tab opens.

luan.herrera, xisigr: Are you still able to repro?

meacer:Which POCs do you run? These two bugs( bug 549724 , bug 634108 )I can't see. I still can reproduce in  bug 677728 's POCs.
(Android chrome 56.0.2924.87)

meacer@, any update on this bug?
- your friendly secondary security sheriff.

I was giving a look at this again and I can reproduce https://bugs.chromium.org/p/chromium/issues/attachmentText?aid=232261 (63.0.3239.111 / Android 6.0.1).

I think I'm actually not the right owner since I don't know how modal dialogs are triggered on Android.

tedchoc: Would you be able to find an appropriate owner? Thanks.

+huayinz@ is making tab modal dialogs work on Android, so sending their way

We already attempt to bring tabs to the foreground when a dialog is shown, so I suspect this is a race condition

TabWebContentsDelegateAndroid#activateContents
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java?q=bringActivityToForeground&sq=package:chromium&dr=CSs&l=349

Maybe we need to prevent tabs being switched while a dialog is present so we could essentially lock it to this tab (only in the case of modal dailogs)?

I wonder if we turn on modal JS dialogs right now, would this problem go away and thus we should just focus on getting that launched?

On desktop we started hiding dialogs from background tabs until the user brings the tab to the foreground because modal dialogs were being abused -- Avi is the expert on this. It would be nice to have parity and do something similar on mobile.

Yes, this is the removal of activation of alert(), https://www.chromestatus.com/feature/6477774290157568 and prompt(), https://www.chromestatus.com/feature/5637107137642496. Gonna do confirm soon.

We should do something similar with Android.

If we turn on tab modal JS dialog, this issue would go away since it brings in Avi's work on removal of activation. So yes, we should just focus on getting that launched.

JS tab modal dialog is launched (see Issue 799334), so this issue won't happen anymore.

*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Nice one luan.herrera@! The VRP panel decided to award $2,000 for this report.

This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

+ awhalley@ (Security TPM) for M72 merge review. I don't see any CL here.

No merge required

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot