Assertion failure: aPos->GetOriginalOffset() < aOriginalEnd (character outside string), at /builds/worker/workspace/build/src/layout/generic/nsTextFrame.cpp:3290
Categories
(Core :: Layout: Text and Fonts, defect, P2)
Tracking
()
People
(Reporter: tsmith, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [post-critsmash-triage][adv-main71+r])
Attachments
(2 files)
135 bytes,
text/html
|
Details | |
47 bytes,
text/x-phabricator-request
|
abillings
:
sec-approval+
|
Details | Review |
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•6 years ago
|
||
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
It's either potentially accessing beyond array boundary, or the assertion is
wrong, I suppose.
That sounds potentially bad. Let's marks this s-s until it is investigated.
Updated•6 years ago
|
Comment 3•5 years ago
|
||
Can you help find an owner to investigate this (potentially) sec-high issue? Thanks!
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
It looks like GetCharacterRectsInRange wants to extend the given range to cluster boundaries; but in this testcase, we have the (unusual) situation that the textframe's content ends mid-cluster. It's possible we should ideally be keeping the 0A71 here, as combining mark, "glued" to the preceding character (>), and not allowing a column break to be forced between them, but I think we may be losing track of that thanks to the script/font boundary that intervenes.
In any case, though, we can make GetCharacterRectsInRange safer and avoid hitting the assertion here by simply testing whether we're already at the end of the content before we try to look ahead for a cluster boundary.
FWIW, I'm not sure there really is much of a security issue here; in this situation, FindClusterEnd will return after advancing a single position, and then GetCharacterRectsInRange will return a rect based on the advance width to this offset (whether or not it really is a cluster end); it won't be accessing beyond the end of the textrun, AFAICS, because there was more text present, it just wasn't part of the current frame's content. So it's possible we might highlight a slightly incorrect area, for example, but it's hard to see much worse happening.
Assignee | ||
Comment 5•5 years ago
|
||
Updated•5 years ago
|
Comment 6•5 years ago
|
||
Hey Dan, referring to comment 4 above from Jonathan, should we downgrade this?
Assignee | ||
Comment 7•5 years ago
|
||
Comment on attachment 9092807 [details]
Bug 1472328 - Check if we're already at the end of the frame's content. r=mats
Security Approval Request
- How easily could an exploit be constructed based on the patch?: I don't think this is readily exploitable, afaict; it could perhaps result in slightly incorrect rendering/UX, but otherwise looks safe to me. (See comment 4.)
(I think we could downgrade the security rating, but to be on the safe side, haven't done so in case I'm overlooking some way this could be exploited further.) - Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
- Which older supported branches are affected by this flaw?: all
- If not all supported branches, which bug introduced the flaw?: None
- Do you have backports for the affected branches?: No
- If not, how different, hard to create, and risky will they be?: Should transplant trivially
- How likely is this patch to cause regressions; how much testing does it need?: Minimal risk
Comment 8•5 years ago
|
||
Comment on attachment 9092807 [details]
Bug 1472328 - Check if we're already at the end of the frame's content. r=mats
sec-approval+ for mozilla-central.
![]() |
||
Comment 9•5 years ago
|
||
https://hg.mozilla.org/integration/autoland/rev/cc64c7f364b796a1696a7012822c3b7af7dfea15
https://hg.mozilla.org/mozilla-central/rev/cc64c7f364b7
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 10•5 years ago
|
||
I tried to verify this issue, but I can't reproduce the initial Assertion failure. I tested on a Windows 10 x64 machine and on Ubuntu machines (16.04 and 18.04) using asan and mc builds from the 29th of June 2018.
Jonathan, could you please give me some additional information that could help me reproduce this?
- Is this issue OS dependent? On what OS should I reproduce/verify this?
- Is this issue reproducible only on certain builds?
- Is this issue still reproducible with the provided test case from the Description? If yes, are there any additional steps needed and not provided in the Description, that could help me in reproducing this issue?
Assignee | ||
Comment 11•5 years ago
|
||
You probably need to use a Debug build to reproduce this; it's a Debug-only assertion, and wouldn't necessarily trigger an error in an Opt/Asan build.
Comment 12•5 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #11)
You probably need to use a Debug build to reproduce this; it's a Debug-only assertion, and wouldn't necessarily trigger an error in an Opt/Asan build.
Thanks, Jonathan! Managed to reproduce this issue on a Firefox 70.0.2 debug build on Ubuntu 18.04 x64. Verified as fixed on the latest Firefox 71 beta 12 debug build on Ubuntu 18.04 x64.
Updated•5 years ago
|
Updated•4 years ago
|
Description
•