Doesn't crash for me on 57.0.2972.0

Could you please provide a testcase that works without user interaction? It should be easy to automate button-clicking Thanks!

mmoroz@, And now I can repro this easly without user interaction.

Deleted: testcase.html 5.9 KB

testcase.html 5.9 KB View Download

This seems like a use-after-free vulnerability. Sorry I cannot get the ASan trace symbolized on Windows.

Deleted: heap-use-after-free_ASan.txt 10.3 KB

heap-use-after-free_ASan.txt 10.3 KB View Download

I reproduced the crash. Over to a media/ owner to take a look. I find it weird that the crashes ends in the WebMediaPlayer when this is a payments request. Is that really expected or did the stack get corrupted? Does this affect stable or just head?

This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

Doesn't repro on stable.

I doubt this has anything to do with media::WebMediaPlayerImpl, since there appears to be no media player involved. I was able to get my local build to crash as well (crash/3ca010fa80000000, SIGSEGV @ 0x00000000), and using objdump this is what I see:

SF0 (chrome + 0x047f8e21)       payments::PaymentRequest::Cancel()	
SF1 (chrome + 0x041f37fc)       payments::PaymentRequestDialog::Cancel()
SF2 (libviews.so + 0x002f92c2)  views::DialogDelegate::Close()
SF3 (libviews.so + 0x002f6dc9)  views::DialogClientView::CanClose()
SF4 (libviews.so + 0x002fb81f)  views::NonClientView::CanClose()
SF5 (libviews.so + 0x002dda0e)  views::Widget::Close()
SF6 (chrome + 0x05d9c0d1)       constrained_window::NativeWebContentsModalDialogManagerViews::Close()
SF7 (chrome + 0x03907499)       web_modal::WebContentsModalDialogManager::DidNavigateMainFrame()
SF8 (chrome + 0x03907714)       web_modal::WebContentsModalDialogManager::WebContentsDestroyed()

Looks like a problem with payments, which makes sense since that is what the repro is testing. Assigning to mathp@ who added the cancel code a few days ago. My guess is that the mojo pipe is already closed when Cancel() tries to use it.

I will note that my crash followed the steps in the initial bug, not the test case in #4. My crash callstack has 100+ entries (on Linux, not Windows), but I only listed the first 9 above.

Any updates on this bug?

A friendly reminder that M57 Beta launch is coming soon on February 2nd! Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix and get it merged into the release branch (2987) ASAP so it gets enough baking time in Dev (before Beta promotion). Thank you!

Since you had to enable --enable-experimental-web-platform-feature flag, this does not affect regular users, so it's not a release blocker.

Desktop implementation of web payments is highly experimental, unreleased, and hidden behind a command-line flag. I appreciate you giving it a go, but please don't be alarmed if it does not work quite right yet.

We're working on a similar issue in  http://crbug.com/683731  with a patch in code review at http://crrev.com/2649683002.

mathp: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4bc50e24731fad2ef8ac320f6347aa60fe70532

commit f4bc50e24731fad2ef8ac320f6347aa60fe70532
Author: mathp <mathp@chromium.org>
Date: Tue Jan 24 05:17:50 2017

[Payments] Improve the closing of the PR dialog.

Various changes around the dialog's disappearance.
* The PaymentRequest object, through its delegate, can now close the
  dialog. This will need to happen if the Mojo pipe is closed by the
  renderer (can happen on failure or success of the PR logic).
* Similarly, when the user closes the dialog through an explicit
  action, the PaymentRequest object will inform the renderer and
  subsequently self destruct (this is not new but is improved)
* Some logic is added to avoid cycles: if the dialog is closing and it
  informs PaymentRequest, PaymentRequest will not ask to close it again
  (and vice versa). This is done through closing the bindings.
* abort() is implemented, it currently simply returns onAbort(true) to
  the caller (to be improved).
* As a result of the Mojo connection closing on navigation, reload,
  etc, the dialog will now close too.
* PaymentRequestDialog is renamed PaymentRequestDialogView and
  implements a new interface, PaymentRequestDialog.
* The tests are now a WidgetObserver to be warned of all possible
  closures of the dialog.

BUG= 683731 ,  679245 
TEST=PaymentRequest interactive_ui_tests

Review-Url: https://codereview.chromium.org/2649683002
Cr-Commit-Position: refs/heads/master@{#445652}

[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/payments/chrome_payment_request_delegate.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/payments/chrome_payment_request_delegate.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/browser_dialogs.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/order_summary_view_controller.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/order_summary_view_controller.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_method_view_controller.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_method_view_controller.h
[rename] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_request_dialog_view.cc
[rename] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_request_dialog_view.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_request_interactive_uitest.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_request_sheet_controller.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_sheet_view_controller.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/payment_sheet_view_controller.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/components/payments/BUILD.gn
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/components/payments/payment_request.cc
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/components/payments/payment_request.h
[modify] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/components/payments/payment_request_delegate.h
[add] https://crrev.com/f4bc50e24731fad2ef8ac320f6347aa60fe70532/components/payments/payment_request_dialog.h

This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

Since this bug needs to enable --enable-experimental-web-platform-features flag, is it qualified for a chromium security reward? Thanks.

I don't think so, but let me loop in some more people to offer their opinion as well. +Zach.

I'm not familiar with the chrome security award procedures, so adding awhalley, but I suspect this won't qualify as it's behind an experimental flag.

If the report caused us to fix a security bug that would have affected stable when we ship the feature then it should be up for consideration. (Unless we're totally sure we would have found it ourselves before shipping).  Adding reward-topanel

Thanks for the report! The panel noted the limited use this bug would be to an attacker, but decided to award $500.

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