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

Issue 765917 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::ImageResourceContent::ImageSize

Project Member Reported by ClusterFuzz, Sep 16 2017

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Null-dereference READ
Crash Address: 0x00000010
Crash State:
  blink::ImageResourceContent::ImageSize
  blink::CachedImageSize
  blink::ImageDocumentParser::Finish
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=499928:499936

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Sep 17 2017

Labels: OS-Android
Cc: msrchandra@chromium.org kkaluri@chromium.org
Components: Blink>Image
Labels: M-63 Test-Predator-Wrong
Owner: malaykeshav@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL did not provide any possible suspects.
Using Code Search for the file, "ImageDocument.cpp" assigning to the concern owner who might be related.

malaykeshav@ -- Could you please look into the issue, kindly re-assign if this is not related to your changes.

Thank You.
 Issue 765717  has been merged into this issue.
Cc: malaykeshav@chromium.org wangxianzhu@chromium.org
Owner: ----
This seems unrelated to my change as it is caused due to an invalid cache state.

Status: Untriaged (was: Assigned)
When you un-assign yourself from a bug you MUST reset the status to Untriaged.
Owner: schenney@chromium.org
Status: Assigned (was: Untriaged)
I'll look at this shortly.
Project Member

Comment 7 by ClusterFuzz, Sep 19 2017

Labels: OS-Linux
Project Member

Comment 8 by ClusterFuzz, Sep 20 2017

Labels: OS-Mac
Cc: -wangxianzhu@chromium.org -kkaluri@chromium.org -msrchandra@chromium.org -malaykeshav@chromium.org hirosh...@chromium.org
Reduced test case.

Crash only happens with these additional gn args:
is_asan = true
enable_ipc_fuzzer = true
is_component_build = false
v8_enable_verify_heap = true

Build content_shell and it only crashes in when run as a layout test, and it only crashes about 80% of the time. Note the image goes in a resources directory.

I have a tentative fix, but review is essential.
fuzz-16.htm
464 bytes View Download
image.gif
163 KB View Download
Project Member

Comment 10 by ClusterFuzz, Oct 1 2017

Components: Blink>HTML Blink>Loader
Labels: Test-Predator-AutoComponents
Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 4 2017

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

commit 834f9a32375933e5a4cef48a06c804f25f82b145
Author: Stephen Chenney <schenney@chromium.org>
Date: Wed Oct 04 13:42:39 2017

Fix null pointer access in ImageDocumentParser

When a image document is re-parented inside an iframe a race
condition exists between the creation of the
ImageContentResource and the parsing of the document.
ImageDocumentParser::Finish requests the CachedImageSize
which accesses a null image pointer inside the image loader.

Standard behavior for image resources is that they be cleared upon
re-parenting, so enforce that for the ImageContentResource as well.

R=hiroshige@chromium.org
BUG= 765917 

Change-Id: I93cea9f3eba879abc66d8b8226c3d56a5e02af1b
Reviewed-on: https://chromium-review.googlesource.com/679074
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506377}
[add] https://crrev.com/834f9a32375933e5a4cef48a06c804f25f82b145/third_party/WebKit/LayoutTests/loader/iframed-image-reparent-crash.html
[add] https://crrev.com/834f9a32375933e5a4cef48a06c804f25f82b145/third_party/WebKit/LayoutTests/loader/resources/image.gif
[modify] https://crrev.com/834f9a32375933e5a4cef48a06c804f25f82b145/third_party/WebKit/Source/core/loader/ImageLoader.cpp

Status: Fixed (was: Assigned)
Project Member

Comment 13 by ClusterFuzz, Oct 5 2017

ClusterFuzz has detected this issue as fixed in range 506368:506383.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_content_shell
Platform Id: windows

Crash Type: Null-dereference READ
Crash Address: 0x00000010
Crash State:
  blink::ImageResourceContent::ImageSize
  blink::CachedImageSize
  blink::ImageDocumentParser::Finish
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=499928:499936
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_content_shell&range=506368:506383

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

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, Oct 5 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: -Test-Predator-AutoComponents Test-Predator-Auto-Components

Sign in to add a comment