Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick ecdec1fb0f42 from chromium #26867

Merged
merged 2 commits into from Dec 10, 2020

Conversation

ppontes
Copy link
Member

@ppontes ppontes commented Dec 7, 2020

Fix text eliding for single-codepoint text with BiDi

This CL is fixing a corner case where RenderText::Elide(...) may
produce a text with more codepoints than the original one. This is
an issue since the breaklists are not resized and the overflow
will lead the render_text code to perfoarm an out-of-bound memory
access by deferencing breaks_.end() while rendering the text. This
is causing chrome to crash.

See crbug/1142020 for details.

The bug was happening when:

  1. The text to elide was a single codepoint
  2. The width of the glyph of the codepoint is larger than the width
    of the ellipsis glyph
  3. Eliding is set to ELIDING_TAIL
  4. The display_rect width will trigger eliding
    (smaller than codepoint width, but larger than ellipsis width)
  5. The render text is set to RTL

A possible solution is to adjust the breaklist but this required
larger refactoring and cannot be a minimal patch to be merge on
other channels.

TBR=msw@chromium.org
(cherry picked from commit e54920751871321474e0b953329c8aedcc8702c3)

Bug: 1142020
Change-Id: I9854651175562ec5f0d0bf7083a8da99fede0e29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522892
Commit-Queue: Etienne Bergeron etienneb@chromium.org
Reviewed-by: Michael Wasserman msw@chromium.org
Cr-Original-Commit-Position: refs/heads/master@{#824878}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537931
Reviewed-by: Etienne Bergeron etienneb@chromium.org
Cr-Commit-Position: refs/branch-heads/4240@{#1450}
Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}

Notes: Security: backported the fix to heap-buffer-overflow in gfx::internal::StyleIterator::GetTextBreakingRange.

@ppontes ppontes requested a review from a team as a code owner December 7, 2020 16:25
@ppontes ppontes added 9-x-y backport-check-skip Skip trop's backport validity checking labels Dec 7, 2020
@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours and removed new-pr 🌱 PR opened in the last 24 hours labels Dec 7, 2020
@codebytere codebytere added the semver/patch backwards-compatible bug fixes label Dec 7, 2020
@ppontes ppontes force-pushed the cherry-pick/9-x-y/chromium/ecdec1fb0f42 branch from 46685f9 to a4b2d1d Compare December 10, 2020 10:41
@jkleinsc jkleinsc merged commit 2d41c02 into 9-x-y Dec 10, 2020
@release-clerk
Copy link

release-clerk bot commented Dec 10, 2020

Release Notes Persisted

Security: backported the fix for https://crbug.com/1142020.

@jkleinsc jkleinsc deleted the cherry-pick/9-x-y/chromium/ecdec1fb0f42 branch December 10, 2020 15:41
@nornagon
Copy link
Member

Nit: it's best if we include the crbug ID in the release notes

This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9-x-y backport-check-skip Skip trop's backport validity checking semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants