New issue
Advanced search Search tips

Issue 734729 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug-Security

Blocked on:
issue 739091



Sign in to add a comment

Compromised renderer can draw form validation bubbles over omnibox

Project Member Reported by lukasza@chromium.org, Jun 19 2017

Issue description

Repro steps:

1. Compromise a renderer process
2. Send ViewHostMsg_ShowValidationMessage asking the browser
   to show a form-validation-bubble at an arbitrary screen offset

Expected behavior: browser process should validate ViewHostMsg_ShowValidationMessage IPC and disallow form-validation-bubbles from appearing outside of web content

Actual behavior: a compromised renderer can affect UI above the security "line of death" (see https://textslashplain.com/2017/01/14/the-line-of-death).  Please see the attached screenshot for an example.

This bug is a follow-up from investigation related to  issue 733940 .
 
FormValidationBubbleOverSecureUI.png
13.2 KB View Download

Comment 1 by est...@chromium.org, Jun 19 2017

Components: Blink>Forms>Validation
Labels: Security_Severity-Low Security_Impact-Stable OS-Linux OS-Windows
Status: Available (was: Untriaged)
I wonder if we should check more things in the browser (rather than in the renderer):

1) page visibility (after r438476 it is being checked in HTMLFormControlElement::updateVisibleValidationMessage: https://chromium.googlesource.com/chromium/src/+/a8e17a3031b6ad69c399e5e04dd0084e577097fc%5E%21/#F0).

2) whether the form validation bubble appears over other frames (this assumes that the problem of frames occluding other frames is solved by intersection observer or other such technology;  in this case I think that the bubble's rectangle should A) be within the bounds of the requesting frame and B) should not occlude other frames).

3) whether the frame requesting the validation bubble is currently being unloaded (after r459701 it is being checked in HTMLFormControlElement::updateVisibleValidationMessage: https://chromium.googlesource.com/chromium/src/+/a896ff44a395a50ab18f5120f20b7eb5a9550247%5E%21/#F2).
Cc: tkent@chromium.org
Silly question: Why do we need to draw the form validation bubbles using special IPCs and a one-off widget, rather than drawing these bubbles together with the rest of web content.  This would avoid all the problems from this bug, as well as the problem of having to hide the bubble after navigating ( issue 733940 ).
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 20 2017

Labels: -Pri-3 Pri-2

Comment 5 by tkent@chromium.org, Jun 20 2017

The reason of the current implementation:
 - It was difficult to implement non-web UI widget in WebKit. We needed to implement validation bubble UI outside of WebKit.
 - Validation bubble UI might depend on OSes.

We forked WebKit four years ago, and validation bubble UI is almost same in all OSes which Chromium supports. So I think we can move the implementation to third_party/WebKit nowadays.  I guess we can use blink::PageOverlay.

I'll discuss if we can add a KR about this to DOM/HTML team Q3 OKR.

Comment 6 by tkent@chromium.org, Jul 4 2017

Blockedon: 739091
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 17 2017

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

commit 2752293b8965ddfdcf7a3780573e795188a62f1f
Author: Kent Tamura <tkent@chromium.org>
Date: Thu Aug 17 01:04:51 2017

ValidationBubbleInRenderer: Enable it by default.

Bug:  432243 ,  597044 ,  734729 ,  736009 ,  736792 ,  739091 
Change-Id: I15a9a24cd86be34dca315f898e556b044bf0acfd
Reviewed-on: https://chromium-review.googlesource.com/607770
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495027}
[modify] https://crrev.com/2752293b8965ddfdcf7a3780573e795188a62f1f/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/2752293b8965ddfdcf7a3780573e795188a62f1f/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

Comment 8 by tkent@chromium.org, Aug 17 2017

Labels: M-62
Owner: tkent@chromium.org
Status: Fixed (was: Available)
Project Member

Comment 9 by sheriffbot@chromium.org, Aug 17 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Release-0-M62
Project Member

Comment 11 by sheriffbot@chromium.org, Nov 23 2017

Labels: -Restrict-View-SecurityNotify allpublic
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

Sign in to add a comment