New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: heap overflow write in render process

Reported by look.wan...@gmail.com, Oct 31 2017

Issue description

VERSION:
Chrome version:Version 62.0.3202.75 (Official Build) (32-bit)
Operating System: Win 10


REPRODUCTION CASE:
(My guess) Because of floating-point computation, it takes about more than 5 minutes to trigger the vulnerability on my old computer(Intel(R) Core(TM) i5-3230M CPU @ 2.60GHz). It should take less than 5 or 3 minutes with a newer cpu.

Open poc.html(attach render process with windbg), wait for about 5 minutes or less, tab will crash.

(2e18.4b4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=0a1c2038 ebx=054fd8e8 ecx=054fd8b0 edx=054fd890 esi=0a1c2008 edi=0a1c2050
eip=578523c0 esp=054fd880 ebp=054fd8e0 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00210202
chrome_child!GetHandleVerifier+0xa1989b:
578523c0 f30f117604      movss   dword ptr [esi+4],xmm6 ds:002b:0a1c200c=????????

Stack info:
 # ChildEBP RetAddr  Args to Child              
00 054fd8e8 5785218c 00000000 0a1c2008 054fdad4 chrome_child!bloat_quad+0x215 (FPO: [Non-Fpo]) (CONV: cdecl) 
01 054fd93c 5785217d 00000000 00000000 054fdad4 chrome_child!add_quads+0x6d (FPO: [Non-Fpo]) (CONV: cdecl) 
02 054fd994 5785217d 00000000 00000000 054fdad4 chrome_child!add_quads+0x5e (FPO: [Non-Fpo]) (CONV: cdecl) 
03 054fd9ec 5785216a 00000000 00000000 054fdad4 chrome_child!add_quads+0x5e (FPO: [Non-Fpo]) (CONV: cdecl) 
04 054fda44 5785216a 00000000 00000000 054fdad4 chrome_child!add_quads+0x4b (FPO: [Non-Fpo]) (CONV: cdecl) 
05 054fda9c 57853831 00000000 00000000 054fdad4 chrome_child!add_quads+0x4b (FPO: [Non-Fpo]) (CONV: cdecl) 
06 054fe7b8 57831470 054fe7c4 05d883c8 0035aed8 chrome_child!`anonymous namespace'::AAHairlineOp::onPrepareDraws+0x435 (FPO: [Non-Fpo]) (CONV: thiscall)
07 054fe7cc 577e7a23 05d883c8 00000000 0038abb0 chrome_child!GrMeshDrawOp::onPrepare+0x17 (FPO: [Non-Fpo]) 


Root cause:
https://cs.chromium.org/chromium/src/third_party/skia/src/gpu/GrBufferAllocPool.cpp?l=394
    void* ptr = INHERITED::makeSpace(vertexSize * vertexCount, // int overflow 
                                     vertexSize,
                                     buffer,
                                     &offset);


 
poc.html
674 bytes View Download

Comment 1 by palmer@chromium.org, Oct 31 2017

Cc: reed@chromium.org
Components: Internals>Skia
Labels: Security_Severity-High Security_Impact-Stable OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows Pri-1
Owner: hcm@chromium.org
Status: Assigned (was: Unconfirmed)
I think the best way to fix this and other integer overflow bugs in Skia is to pull in base/numerics (a headers-only dependency) and use `CheckMul`, `CheckAdd`, and so on.
Project Member

Comment 2 by ClusterFuzz, Oct 31 2017

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5973507480748032.
Project Member

Comment 3 by sheriffbot@chromium.org, Nov 1 2017

Labels: M-62

Comment 4 by est...@chromium.org, Nov 11 2017

hcm, is there someone who can take a look at this? Thanks!
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 16 2017

hcm: Uh oh! This issue still open and hasn't been updated in the last 15 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

Comment 6 by hcm@chromium.org, Nov 16 2017

Cc: hcm@chromium.org
Labels: -OS-Linux -OS-Android -OS-Chrome -OS-Mac -OS-Fuchsia
Owner: bsalomon@chromium.org
I cannot get this to crash on Linux or Mac, will try debug on my Win system tonight. Adding Brian for a look at the Ganesh ops in question...

Should this really be P1?

Comment 7 by reed@chromium.org, Nov 16 2017

Cc: bsalomon@chromium.org mtklein@chromium.org
Owner: reed@google.com
@ hcm@chromium.org
"Because of floating-point computation, it takes about more than 5 minutes to trigger the vulnerability."
Maybe you need wait for 2 or 3 minutes.

Comment 9 Deleted

I can reproduce the vul on my Android phone(CPU:MSM8992),
which took about 10 mins.

Comment 11 by hcm@chromium.org, Nov 17 2017

Labels: -Pri-1 Pri-2
I have been letting each test go for 10+ minutes.  Tried testing on a Win10 system (Intel Core i3-6100U CPU @ 2.30GHz) last night, but was still not able to repro.  

Nonetheless, we know there are potential improvements we can make in the code to avoid overflows, so I think reed@ is going to pursue.


With 32-bit chrome?
And should use official release of chrome instead of self-build version.
And open "chrome://gpu/",
make sure that there is "Canvas: Hardware accelerated" (Canvas is usually hardware accelerated on windows)

Comment 15 by hcm@chromium.org, Nov 17 2017

Ah good point, it is most certainly *not* 32-bit since Windows systems with appropriate specs/memory should have upgraded to 64-bit some time ago.  I will grab a 32-bit install and hopefully we'll get our repro.
Labels: -Pri-2 -M-62 M-64 OS-Android Pri-1
Any news? We don't want to let a High severity bug live for too long, and it has been 11 days since the last update. Thanks!

Comment 17 by hcm@chromium.org, Nov 30 2017

I was eventually able to repro an unresponsive tab on a Win10 system today with the 32-bit install, thanks OP for the details.


Project Member

Comment 18 by sheriffbot@chromium.org, Dec 1 2017

reed: Uh oh! This issue still open and hasn't been updated in the last 30 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
hcm@ Has there been progress on this since you were able to repro? Would you be a good owner going forward?
Project Member

Comment 20 by sheriffbot@chromium.org, Dec 31 2017

Labels: Deadline-Exceeded
We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue?

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

Comment 21 by bugdroid1@chromium.org, Jan 17 2018

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/fe266c2bce2b8ac4ef953f16c8e1a7801da9c57d

commit fe266c2bce2b8ac4ef953f16c8e1a7801da9c57d
Author: Mike Reed <reed@google.com>
Date: Wed Jan 17 19:31:16 2018

use safemath::mull for buffer sizes

Bug:780104
Change-Id: Ic683abd9c7d15ebb01b6e5d40dbeb6e76f102eff
Reviewed-on: https://skia-review.googlesource.com/95760
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Reed <reed@google.com>

[modify] https://crrev.com/fe266c2bce2b8ac4ef953f16c8e1a7801da9c57d/src/gpu/GrBufferAllocPool.cpp

reed: Is the fix in comment #21 final? Can this be closed?

Comment 23 by reed@google.com, Jan 23 2018

Status: Fixed (was: Assigned)
Thanks!

Comment 25 by hcm@chromium.org, Jan 24 2018

Note this change did not make the M65 cut.  Until we decide to cherry-pick, it should be present in all 66+ builds.

That said, I was still able to repro on a fresh 66 Win32 Canary on one of my Win10 test systems..so there may be more to address here.  Interested in a test from the reporter to verify...


Labels: reward-topanel Merge-Request-65
Thanks. And requesting 65 merge.
Cc: awhalley@chromium.org
+ awhalley@ for M65 merge review
Cc: abdulsyed@chromium.org
Is this only need merge to M64? If yes, pls request a merge to M64. Thank you.
Cc: -mtklein@chromium.org
Labels: M-65
govind@ - good for 65, and let's start with just that.
Project Member

Comment 31 by sheriffbot@chromium.org, Jan 30 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M65 branch 3325 ASAP so we can pick it up for next M65 dev release. Thank you.
Project Member

Comment 33 by bugdroid1@chromium.org, Jan 30 2018

Labels: merge-merged-m65
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/8cf17a8c72b19444d51d3d064c69fcc9faaba1c9

commit 8cf17a8c72b19444d51d3d064c69fcc9faaba1c9
Author: Mike Reed <reed@google.com>
Date: Tue Jan 30 19:57:02 2018

use safemath::mull for buffer sizes

Bug:780104
Change-Id: Ic683abd9c7d15ebb01b6e5d40dbeb6e76f102eff
Reviewed-on: https://skia-review.googlesource.com/95760
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Reed <reed@google.com>
(cherry picked from commit fe266c2bce2b8ac4ef953f16c8e1a7801da9c57d)
Reviewed-on: https://skia-review.googlesource.com/101780
Reviewed-by: Mike Reed <reed@google.com>

[modify] https://crrev.com/8cf17a8c72b19444d51d3d064c69fcc9faaba1c9/src/gpu/GrBufferAllocPool.cpp

Seems like this is already merged to M65 per comment #33. If nothing is pending, pls remove "Merge-Approved-65" label. Thank you.

Comment 35 by hcm@chromium.org, Jan 31 2018

Labels: -merge-merged-m65
Yes it was merged (as an aside, why doesn't bugdroid remove merge-approved when it applies merge-merged?)
look.wangluke@ - are you able to reproduce in a version of Chrome with the fix from #21?
Labels: -Merge-Approved-65 merge-merged-m65
Project Member

Comment 38 by sheriffbot@chromium.org, Feb 8

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Nice one! The Chrome VRP panel decided to award $1,000 for this report.
Labels: -reward-topanel reward-unpaid reward-1000
*** 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.
*********************************
Labels: -reward-unpaid reward-inprocess
Labels: Release-0-M65
Labels: CVE-2018-6062
Project Member

Comment 44 by sheriffbot@chromium.org, Mar 27

Labels: -M-64
Labels: CVE_description-missing
Project Member

Comment 46 by sheriffbot@chromium.org, May 2

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