New issue
Advanced search Search tips

Issue 880009 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference WRITE in blink::PresentationController::SetPresentation

Project Member Reported by ClusterFuzz, Sep 3

Issue description

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

Fuzzer: inferno_twister
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x000000000040
Crash State:
  blink::PresentationController::SetPresentation
  blink::Presentation::Create
  blink::NavigatorPresentation::presentation
  
Sanitizer: address (ASAN)

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: pnangunoori@chromium.org
Labels: M-69 Test-Predator-Wrong
Owner: jbroman@chromium.org
Status: Assigned (was: Untriaged)
As per the  Issue 850884  owner assigning this issue to @jbroman.
@jbroman -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.
Thanks.
Cc: jbroman@chromium.org
Components: Blink>PresentationAPI
Owner: mlamouri@chromium.org
Hrm. Apparently this test is able to reach Presentation::Create before PresentationController::ProvideTo is called for that LocalFrame, which should be early in the frame's lifetime. (Determined with "clusterfuzz reproduce" + debug_renderer.) Since it expects it to be non-null, the program crashes.

Not caused by my change, so bouncing to mlamouri@ from modules/presentation/OWNERS to have them look at whether NavigatorPresentation ought to deal with this, or PresentationController ought to be created earlier, or something else.

I'd have looked in a little more detail, but I'm on vacation and probably shouldn't have done this much. ;)
Components: Blink>Loader
The test case is iterating over (and redefining?) properties of window and is not specific to the Presentation API.  Based on some review of the code, it seems like either one of two things is happening:

1. The Presentation API is disabled for the document, which could only happen with command line flags IIUC.

2. a LocalFrame without supplements installed is observable from its script execution context.  It looks like supplements are installed before the navigation commits.  That seems wrong, and if this is happening then it would break several other APIs that require LocalFrame supplements.

We can address situation #1 with a null check, but ClusterFuzz may be hitting situation #2 - I can't find the commandline for the testcase at the link given.

Can anyone on the loader team provide insight into whether render modules should be designed to handle LocalFrames without supplements installed?

The command line is:
```
/mnt/scratch0/clusterfuzz/slave-bot/builds/chromium-browser-asan_linux-release_4392242b7f59878a2775b4607420a2b37e17ff13/revisions/asan-linux-release-589450/content_shell --user-data-dir=/mnt/scratch0/tmp/user_profile_0 --run-web-tests --dump-render-tree --use-gl=swiftshader --disable-in-process-stack-traces /mnt/scratch0/clusterfuzz/slave-bot/inputs/fuzzer-testcases/fast/forms/calendar-picker/fuzz-69.html
```

FWIW, the test uses `window.internals.pagePopupWindow`. I haven't checked if this is the root cause of the issue but clusterfuzz testcase using internals are suspicious.
Cc: haraken@chromium.org dcheng@chromium.org japhet@chromium.org
Adding a few folks who might have ideas on the question in Comment #3:

"Can anyone on the loader team provide insight into whether render modules should be designed to handle LocalFrames without supplements installed?"


Project Member

Comment 6 by ClusterFuzz, Sep 8

ClusterFuzz has detected this issue as fixed in range 589774:589775.

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

Fuzzer: inferno_twister
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x000000000040
Crash State:
  blink::PresentationController::SetPresentation
  blink::Presentation::Create
  blink::NavigatorPresentation::presentation
  
Sanitizer: address (ASAN)

Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=589774:589775

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

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 7 by ClusterFuzz, Sep 8

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Would you help me understand how it's possible that Presentation::Create gets called before ModulesInitializer::InstallSupplements gets called?

(That seems wrong to me.)

Re: C#8

I wasn't able to find a way for that to happen by looking at the code.  The only time I've seen this crash hit is through ClusterFuzz, and it seems racy (for example this crash stopped reproducing on CF).

If we can get a reliable repro we might be able to get more details.


Cc: kkaluri@chromium.org
 Issue 882411  has been merged into this issue.
Project Member

Comment 11 by ClusterFuzz, Sep 12

Labels: OS-Mac
Project Member

Comment 12 by ClusterFuzz, Sep 15

Labels: Needs-Feedback
ClusterFuzz testcase 5565827821338624 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.

Sign in to add a comment