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

Issue 852435 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Integer-overflow in blink::LayoutFrameSet::UpdateLayout

Project Member Reported by ClusterFuzz, Jun 13 2018

Issue description

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

Fuzzer: ifratric-browserfuzzer-v3
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::LayoutFrameSet::UpdateLayout
  blink::LocalFrameView::LayoutFromRootObject
  blink::LocalFrameView::LayoutOrthogonalWritingModeRoots
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=482916:482982

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Jun 13 2018

Components: Blink>Internals Blink>Layout
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Jun 13 2018

Cc: hayato@chromium.org r...@opera.com iclell...@chromium.org
Labels: Test-Predator-Auto-CC
Automatically adding ccs based on suspected regression changelists:

Move dom/shadow/ files to dom/ by hayato@chromium.org - https://chromium.googlesource.com/chromium/src/+/a703ff1bc9db59827740a249bdcbbf8c6e8c1f1c

Separate initial style and viewport/ICB style. by rune@opera.com - https://chromium.googlesource.com/chromium/src/+/334b4846828ceb697785b466619a1f1b96592a3b

Set frame policy correctly in newly created renderer proxies by iclelland@chromium.org - https://chromium.googlesource.com/chromium/src/+/098da75cd0415c2e21a7ee78e47a75abdfabf461

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.
Labels: Test-Predator-Wrong-CLs
This is an actual integer overflow bug, though not related to any of the suspected CLs. (Sorry, Predator)

The not-quite-minimized testcase is a script which inserts a

<frameset rows="1073741824,-1"></frameset>

into an HTML document. (Not sure if this is actually valid, but HTML5 obsoletes the element, with very few requirements left on its processing anymore)

HTML4.01 says "Absolute lengths that do not sum to 100% of the real available space should be adjusted by the user agent. ... When overspecified, each view should be reduced according to its specified proportion of the total space."

That is causing the first column width, (2^30 px), to be multiplied by the available space (912px) in an int (this is apparently *not* promoted to long long for purposes of the multiplication)

As far as I can see in git blame, this has been present since 2006.
Owner: iclell...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 14 2018

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

commit 47ef1c5d6dc6f2a604ba43827b9d81c2ac0dc6c0
Author: Ian Clelland <iclelland@chromium.org>
Date: Thu Jun 14 02:26:37 2018

Fix integer overflow in frameset layout

Existing code was multiplying ints in a context where an intermediate
result could overflow the 32-bit container. Change to explicitly use a
64-bit long long for the intermediate product to avoid undefined
behaviour.

Found by UBSan / clusterfuzz

Bug:  852435 
Change-Id: I683eee6eda51f40e7f165c0a55111fba623c2ec9
Reviewed-on: https://chromium-review.googlesource.com/1099756
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567097}
[modify] https://crrev.com/47ef1c5d6dc6f2a604ba43827b9d81c2ac0dc6c0/third_party/blink/renderer/core/layout/layout_frame_set.cc

Project Member

Comment 6 by ClusterFuzz, Jun 14 2018

ClusterFuzz has detected this issue as fixed in range 567089:567097.

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

Fuzzer: ifratric-browserfuzzer-v3
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  blink::LayoutFrameSet::UpdateLayout
  blink::LocalFrameView::LayoutFromRootObject
  blink::LocalFrameView::LayoutOrthogonalWritingModeRoots
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=482916:482982
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_chrome&range=567089:567097

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

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, Jun 14 2018

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

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

Sign in to add a comment