vabr@, could you please help to find an owner for that?

(Cc-ing CM API experts)

mmoroz@ -- do you know how much UBSAN's diagnostics can be trusted?

It says that the cast in the code snippet [1]:

const blink::WebCredential& input;
...
if (input.isPasswordCredential()) {
...
        static_cast<const blink::WebPasswordCredential&>(input)
...

The down-cast of |input| is invalid. But as long as |isPasswordCredential| is not crazy, that down-cast is valid.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/components/password_manager/content/public/cpp/type_converters.cc&q=components/password_manager/content/public/cpp/type_converters.cc&sq=package:chromium&type=cs&l=80-87

vabr@, I've heard that it may have false positives. Adding Kostya and Mike to take a look.

It seems pretty sure of a vptr value. How can this be false positive?

Does this issue reproduce? What does debugger say?

I did not manage to reproduce, so no debugger. But I now maybe understand the issue:
WebCredential is a wrapper around PlatformCredential. If WebCredential::isPasswordCredential says true, that means that that WebCredential wraps a PlatformPasswordCredential, but probably not that the wrapper itself is a WebPasswordCredential. I'll investigate more and will try to fix this.

So the code creates WebCredential around PlatformPasswordCredential somewhere, instead of creating WebPasswordCredential. This easily reproduces also by submitting the password form on https://w3c.github.io/webappsec/demos/credential-management/.

So UBSAN is correct, and found a bug in our code. Great job UBSAN and your creators!

I'll follow up with a patch of our code shortly.

The issue is that https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/platform/exported/WebCredential.cpp&l=17 does object slicing:
The create() method constructs a WebPasswordCredential variable and calls return on it. But the return type is WebCredential, so the compiler uses WC's copy constructor to slice away the sub-class part, and returns a proper WebCredential.

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

commit 9818ab57e995473e2d41d5bb40b6a9abd2682151
Author: vabr <vabr@chromium.org>
Date: Wed May 11 17:01:30 2016

Avoid object slicing in WebCredential::create

WebCredential::create currently returns a WebCredential. It tries to create one
of its subclasses, WebPasswordCredential or WebFederatedCredential,
respectively, but those object get sliced to their common base upon returning,
because a function returning A cannot return B even if B is a subclass of A.

To fix that, this CL changes create() to return a pointer to a WebCredential.
This allows creating subclasses of it while keeping the return type as
declared.

R=mkwst@chromium.org
BUG= 610646 

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

[modify] https://crrev.com/9818ab57e995473e2d41d5bb40b6a9abd2682151/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
[modify] https://crrev.com/9818ab57e995473e2d41d5bb40b6a9abd2682151/third_party/WebKit/Source/platform/exported/WebCredential.cpp
[modify] https://crrev.com/9818ab57e995473e2d41d5bb40b6a9abd2682151/third_party/WebKit/public/platform/WebCredential.h

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

commit e35fb1fb6c25186d0f3758d080fe2a2d48e77043
Author: vabr <vabr@chromium.org>
Date: Thu May 12 10:59:48 2016

Fix style in Credential Manager code

This CL addresses post-landing comments from
https://codereview.chromium.org/1967693003/.

R=yutak@chromium.org
TBR=mkwst@chromium.org
BUG= 610646 

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

[modify] https://crrev.com/e35fb1fb6c25186d0f3758d080fe2a2d48e77043/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
[modify] https://crrev.com/e35fb1fb6c25186d0f3758d080fe2a2d48e77043/third_party/WebKit/Source/platform/exported/WebCredential.cpp

Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Request-XX label, where XX is the Chrome milestone.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz

Requesting merge of r392959 to M51 (2704).

Requesting merge of r392959 to M50 (2661).

[Automated comment] Request affecting a post-stable build (M50), manual review required.

Your change meets the bar and is auto-approved for M51 (branch: 2704)

[Automated comment] Request affecting a post-stable build (M50), manual review required.

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

commit a11ede2c9225b66ee9403f5bf26eceb11987afb6
Author: Vaclav Brozek <vabr@chromium.org>
Date: Fri May 13 11:43:23 2016

Avoid object slicing in WebCredential::create

WebCredential::create currently returns a WebCredential. It tries to create one
of its subclasses, WebPasswordCredential or WebFederatedCredential,
respectively, but those object get sliced to their common base upon returning,
because a function returning A cannot return B even if B is a subclass of A.

To fix that, this CL changes create() to return a pointer to a WebCredential.
This allows creating subclasses of it while keeping the return type as
declared.

R=mkwst@chromium.org
BUG= 610646 

Review-Url: https://codereview.chromium.org/1967693003
Cr-Commit-Position: refs/heads/master@{#392959}
(cherry picked from commit 9818ab57e995473e2d41d5bb40b6a9abd2682151)

Review URL: https://codereview.chromium.org/1976943002 .

Cr-Commit-Position: refs/branch-heads/2704@{#533}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/a11ede2c9225b66ee9403f5bf26eceb11987afb6/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
[modify] https://crrev.com/a11ede2c9225b66ee9403f5bf26eceb11987afb6/third_party/WebKit/Source/platform/exported/WebCredential.cpp
[modify] https://crrev.com/a11ede2c9225b66ee9403f5bf26eceb11987afb6/third_party/WebKit/public/platform/WebCredential.h

go/betabuilders look fine after #20.

I am withdrawing the merge request for 50 based on these grounds:
(1) Credential manager API is not launched in 50. This code can only be triggered through Credential manager API.
(2) Version 51 enters stable really soon.
(3) I am not able to merge anything next week.

 Issue 605477  has been merged into this issue.

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

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

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