New issue
Advanced search Search tips

Issue 792306 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug-Security



Sign in to add a comment

Use-of-uninitialized-value in bool blink::FastParseColorInternal<unsigned char>

Project Member Reported by ClusterFuzz, Dec 6 2017

Issue description

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

Fuzzer: libFuzzer_css_parser_fast_paths_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  bool blink::FastParseColorInternal<unsigned char>
  blink::CSSParserFastPaths::ParseColor
  blink::CSSParserFastPaths::MaybeParseValue
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=521690:521711

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
 
Cc: hichris...@gmail.com
Components: Blink>CSS
Labels: OS-Android OS-Chrome OS-Mac OS-Windows
Owner: ericwilligers@chromium.org
The inlined function checks the fifth character after only verifying that there are at least 4 characters.

https://chromium.googlesource.com/chromium/src/+/106b2819bfca32edee99e803364395453b31a2ca
template <typename CharacterType>
static inline bool MightBeRGBOrRGBA(const CharacterType* characters,
                                    unsigned length) {
  if (length < 4)                    // <--------------------- Four or more chars
    return false;
  return IsASCIIAlphaCaselessEqual(characters[0], 'r') &&
         IsASCIIAlphaCaselessEqual(characters[1], 'g') &&
         IsASCIIAlphaCaselessEqual(characters[2], 'b') &&
         (characters[3] == '(' ||
          (IsASCIIAlphaCaselessEqual(characters[3], 'a') &&
           characters[4] == '('));    // <---------------------- Fifth Character
}
Status: Assigned (was: Untriaged)
Labels: -Security_Severity-Medium Security_Severity-Low
Setting to Security_Severity-Low, as I don't think would be useful to an attacker as anything more than a single-byte compare.
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 6 2017

Labels: Pri-2
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 6 2017

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

commit 7a5ddbe0167e6c988e8f0a4a4314ee8377cd21a4
Author: Eric Willigers <ericwilligers@chromium.org>
Date: Wed Dec 06 21:41:04 2017

CSS rgb(a) parsing: avoid uninitialized-value

We check that the input has at least 5 characters before reading
characters[4].

An rgb(a) color contains at least 3 numbers or percentages,
plus separator characters, so the length of a valid rgb(a) color
will be well over 5.

TBR=nainar@chromium.org
BUG= 792306 

Change-Id: Iceb49992ee7c5da346e2f64cbc7ccb82ad619f81
Reviewed-on: https://chromium-review.googlesource.com/811784
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522203}
[modify] https://crrev.com/7a5ddbe0167e6c988e8f0a4a4314ee8377cd21a4/third_party/WebKit/Source/core/css/parser/CSSParserFastPaths.cpp

Project Member

Comment 6 by ClusterFuzz, Dec 7 2017

ClusterFuzz has detected this issue as fixed in range 522181:522219.

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

Fuzzer: libFuzzer_css_parser_fast_paths_fuzzer
Job Type: libfuzzer_chrome_msan
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  bool blink::FastParseColorInternal<unsigned char>
  blink::CSSParserFastPaths::ParseColor
  blink::CSSParserFastPaths::MaybeParseValue
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=521690:521711
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_msan&range=522181:522219

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.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 7 by ClusterFuzz, Dec 7 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 5054769474568192 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 8 by sheriffbot@chromium.org, Dec 7 2017

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

Comment 9 by sheriffbot@chromium.org, Mar 15 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