New issue
Advanced search Search tips

Issue 764921 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security



Sign in to add a comment

Stack-buffer-overflow in test_runner::EventSender::SendCurrentTouchEvent

Project Member Reported by ClusterFuzz, Sep 13 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6171791172304896

Fuzzer: inferno_layout_test_fuzzer
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Stack-buffer-overflow WRITE {*}
Crash Address: 0x1839bb20
Crash State:
  test_runner::EventSender::SendCurrentTouchEvent
  test_runner::EventSenderBindings::TouchStart
  base::internal::Invoker<base::internal::BindState<bool
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6171791172304896

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Sep 14 2017

Labels: M-61
Project Member

Comment 2 by sheriffbot@chromium.org, Sep 14 2017

Labels: Pri-1

Comment 3 by mea...@chromium.org, Sep 15 2017

Components: Blink>Input
Owner: hirosh...@chromium.org
Status: Assigned (was: Untriaged)
hiroshige: Can you please take a look? Thanks.
What is the context for assigning to me?
I'm not familiar with the code that appears in the stack trace.
Components: -Blink>Input Test
Labels: -Security_Impact-Stable Security_Impact-None
Owner: mustaq@chromium.org
Test runner only issue, assigning to mustaq@ who seems to have fixed stuff in this code.

Comment 6 by mustaq@chromium.org, Sep 27 2017

Cc: dtapu...@chromium.org mustaq@chromium.org
Components: -Test Blink>Layout
Owner: eirage@chromium.org
Ella, could you please take a look?

Comment 7 by eirage@chromium.org, Sep 27 2017

the test refresh every 2 second, and add 2 touch points each time. 
It crash on EventSender::SendCurrentTouchEvent's DCHECK: touch_point_.size() reach  kTouchesLengthCap(16).
Is that expected?

Comment 8 by mustaq@chromium.org, Sep 27 2017

Refreshing every two seconds in not a realistic layout test scenario.  But just for sake of preventing the buffer overflow, let's prevent adding touch points beyond kTouchesLengthCap.
Change the dcheck on https://cs.chromium.org/chromium/src/content/shell/test_runner/event_sender.cc?q=event_sender.cc&sq=package:chromium&l=2281

to be:

if (touches_.size() > WebTouchEvent::kTouchesLengthCap) {
  args->ThrowError();
  return;
}

Comment 10 by e...@chromium.org, Sep 28 2017

Components: -Blink>Layout Blink>Infra
(input crash in test runner, not layout related. moving to Blink>Infra)
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 28 2017

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

commit f69b51b142b5285a4687074d6eb86908284bc84f
Author: Ella Ge <eirage@chromium.org>
Date: Thu Sep 28 16:30:45 2017

Do not add touchpoint when reach kTouchesLengthCap

In this cl, we throw error in EventSender::AddTouchPoint
to prevent touch_point_.size() exceed kTouchesLengthCap.

Bug:  764921 
Change-Id: Ifbb8db421f1a523e91a845284656b17d6269bb40
Reviewed-on: https://chromium-review.googlesource.com/687809
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505047}
[modify] https://crrev.com/f69b51b142b5285a4687074d6eb86908284bc84f/content/shell/test_runner/event_sender.cc

Status: Fixed (was: Assigned)
Project Member

Comment 13 by ClusterFuzz, Sep 29 2017

ClusterFuzz has detected this issue as fixed in range 505032:505082.

Detailed report: https://clusterfuzz.com/testcase?key=6171791172304896

Fuzzer: inferno_layout_test_fuzzer
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Stack-buffer-overflow WRITE {*}
Crash Address: 0x18d5bd40
Crash State:
  test_runner::EventSender::SendCurrentTouchEvent
  test_runner::EventSenderBindings::TouchStart
  base::internal::Invoker<base::internal::BindState<bool
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Fixed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=505032:505082

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6171791172304896

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 14 by ClusterFuzz, Sep 29 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 6171791172304896 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 15 by sheriffbot@chromium.org, Sep 29 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 16 by sheriffbot@chromium.org, Jan 5 2018

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