New issue
Advanced search Search tips

Issue 658555 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in pp::MacroExpander::pushMacro

Project Member Reported by ClusterFuzz, Oct 22 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6253501774823424

Fuzzer: afl_angle_translator_fuzzer
Job Type: afl_chrome_asan
Platform Id: linux

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x60d00000180c
Crash State:
  pp::MacroExpander::pushMacro
  pp::MacroExpander::lex
  pp::Preprocessor::lex
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=afl_chrome_asan&range=420327:420425

Minimized Testcase (3.20 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94hno2qSRtWMvqNJbWT3N-dK5oxuR623MIoCx8IS7_h06LMXkV-e0ehPIucRJwJ5VzF1KSjox-KfRG2r9DU1n1-KZU8HpB_8Rjyq04LXtnOixM-0NS4CN3YkuIutF4IxawMNYVAXJXzw37ePh3wnbpYB4QDdQ?testcase_id=6253501774823424

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Project Member

Comment 1 by sheriffbot@chromium.org, Oct 23 2016

Labels: M-55
Project Member

Comment 2 by sheriffbot@chromium.org, Oct 23 2016

Labels: ReleaseBlock-Stable
Project Member

Comment 3 by sheriffbot@chromium.org, Oct 23 2016

Labels: Pri-1

Comment 4 by ta...@google.com, Oct 25 2016

Components: Internals>GPU>ANGLE
Owner: cwallez@chromium.org
Status: Assigned (was: Untriaged)
cwallez@, could you take a look at this? I think this line breaks:

macro.expansionCount++;

https://chromium.googlesource.com/angle/angle.git/+/d2f195b5a4b266be817193ed004c26535a32723a%5E%21/#F4

Thanks

Comment 5 by gov...@chromium.org, Oct 26 2016

**** Bulk edit -  please ignore if not applicable ****

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

Comment 6 by bugdroid1@chromium.org, Oct 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/449a803040d9a6c44fc88c4d91b136292791e326

commit 449a803040d9a6c44fc88c4d91b136292791e326
Author: Corentin Wallez <cwallez@chromium.org>
Date: Wed Oct 26 12:05:54 2016

MacroExpander: bump expansionCount before peeking for "("

BUG= 658555 

Change-Id: I578b8aff37a116fd7b2b387388311a27bb8a2809
Reviewed-on: https://chromium-review.googlesource.com/403848
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

[modify] https://crrev.com/449a803040d9a6c44fc88c4d91b136292791e326/src/compiler/preprocessor/MacroExpander.cpp
[modify] https://crrev.com/449a803040d9a6c44fc88c4d91b136292791e326/src/tests/preprocessor_tests/define_test.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 27 2016

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

commit a16bd358e88a73acb35843f44719783f24087234
Author: geofflang <geofflang@chromium.org>
Date: Thu Oct 27 21:18:24 2016

Roll ANGLE af7f301..1d2c41d

https://chromium.googlesource.com/angle/angle.git/+log/af7f301..1d2c41d

BUG= 658555 , chromium:659326 , 540829 ,655247

TBR=jmadill@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

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

[modify] https://crrev.com/a16bd358e88a73acb35843f44719783f24087234/DEPS

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 27 2016

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

commit 9bfaa6948251aae2a4f2b8ca178e8c9c3f5e78c3
Author: jam <jam@chromium.org>
Date: Thu Oct 27 21:49:44 2016

Revert of Roll ANGLE af7f301..1d2c41d (patchset #1 id:1 of https://codereview.chromium.org/2457883002/ )

Reason for revert:
breaks build
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/11909/steps/compile/logs/stdio

Original issue's description:
> Roll ANGLE af7f301..1d2c41d
>
> https://chromium.googlesource.com/angle/angle.git/+log/af7f301..1d2c41d
>
> BUG= 658555 , chromium:659326 , 540829 ,655247
>
> TBR=jmadill@chromium.org
>
> TEST=bots
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
>
> Committed: https://crrev.com/a16bd358e88a73acb35843f44719783f24087234
> Cr-Commit-Position: refs/heads/master@{#428138}

TBR=jmadill@chromium.org,geofflang@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 658555 , chromium:659326 , 540829 ,655247

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

[modify] https://crrev.com/9bfaa6948251aae2a4f2b8ca178e8c9c3f5e78c3/DEPS

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 30 2016

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

commit 988aa562e76cacc7b6b39d9f7b33c25f5b7824f8
Author: qiankun.miao <qiankun.miao@intel.com>
Date: Sun Oct 30 09:14:46 2016

Roll ANGLE af7f301..705a919

https://chromium.googlesource.com/angle/angle.git/+log/af7f301..705a919

BUG= chromium:659892 ,  540829 , 655247,  chromium:639760 ,  chromium:659326 ,  chromium:659326 ,  658555 

TEST=bots

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

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

[modify] https://crrev.com/988aa562e76cacc7b6b39d9f7b33c25f5b7824f8/DEPS

Project Member

Comment 10 by ClusterFuzz, Oct 31 2016

ClusterFuzz has detected this issue as fixed in range 428621:428622.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6253501774823424

Fuzzer: afl_angle_translator_fuzzer
Job Type: afl_chrome_asan
Platform Id: linux

Crash Type: Heap-use-after-free READ 4
Crash Address: 0x60d00000180c
Crash State:
  pp::MacroExpander::pushMacro
  pp::MacroExpander::lex
  pp::Preprocessor::lex
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=afl_chrome_asan&range=420327:420425
Fixed: https://cluster-fuzz.appspot.com/revisions?job=afl_chrome_asan&range=428621:428622

Minimized Testcase (3.20 Kb): https://cluster-fuzz.appspot.com/download/AMIfv94hno2qSRtWMvqNJbWT3N-dK5oxuR623MIoCx8IS7_h06LMXkV-e0ehPIucRJwJ5VzF1KSjox-KfRG2r9DU1n1-KZU8HpB_8Rjyq04LXtnOixM-0NS4CN3YkuIutF4IxawMNYVAXJXzw37ePh3wnbpYB4QDdQ?testcase_id=6253501774823424

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md 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 11 by ClusterFuzz, Oct 31 2016

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

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

Comment 12 by sheriffbot@chromium.org, Nov 1 2016

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

Comment 13 by sheriffbot@chromium.org, Nov 3 2016

Labels: Merge-Request-55

Comment 14 by dimu@chromium.org, Nov 3 2016

Labels: -Merge-Request-55 Merge-Review-55 Hotlist-Merge-Review
[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.
Labels: -Hotlist-Merge-Review -M-55 -Security_Impact-Beta -ReleaseBlock-Stable -Merge-Review-55
This fix is hard to merge to M55 because:
 - It depends on a previous fix that wasn't merged to M55
 - It requires a DEPS roll

In addition it is very unlikely to be hit by users, only someone knowing about the bug can trigger it.

Removing all the merging-related labels.
cwallez@: Unfortunately the threat for security bugs is that it will be deliberately triggered by an attacker who knows about it :-(  Would it be too much work / risky to consider an M55 specific fix?
Backporting the fix would be at least a couple hours for the tests and the added DEPS roll. My bigger concern is that this bug is part of a number of other security bugs that are being found by this fuzzer. Backporting one (two in this case) of the fixes is ok, but not backporting a dozen or more fixes.

This combined with the fact that these bugs have been present since the beginning of time make me reticent to do merges for them. WDYT?
FYI corentin, we don't DEPS roll to branches anymore since a while back. We just push to chromium/#### in ANGLE and buildspec picks up the appropriate branch.
Cc: och...@chromium.org
Labels: Security_Impact-Stable M-56 M-55
ochang@, any thoughts on #17?
Labels: -M-55
Labels: Release-0-M56
Project Member

Comment 22 by sheriffbot@chromium.org, Feb 6 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