Closed
Bug 1315986
Opened 8 years ago
Closed 8 years ago
AddressSanitizer: global-buffer-overflow on address 0x at pc 0x bp 0x sp 0x
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox-esr45 | --- | unaffected |
firefox50 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | - | fixed |
People
(Reporter: cbook, Assigned: jfkthame)
References
(Blocks 1 open bug, )
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
masayuki
:
review+
|
Details | Diff | Splinter Review |
Bughunter reports reports lots of AddressSanitizer: global-buffer-overflow on address 0x at pc 0x bp 0x sp 0x on sites like
https://twitter.com/search?q=Golden%20Tate&src=tyah
http://www.espn.com/nfl/scoreboard
etc
seems nightly only and graphics related (no idea if related to bug 1315979)
Johnathan, could you take a look, thanks!
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(jfkthame)
Assignee | ||
Comment 1•8 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #0)
> Created attachment 8808626 [details]
> stack
>
> Bughunter reports reports lots of AddressSanitizer: global-buffer-overflow
> on address 0x at pc 0x bp 0x sp 0x on sites like
>
> https://twitter.com/search?q=Golden%20Tate&src=tyah
> http://www.espn.com/nfl/scoreboard
> etc
>
> seems nightly only and graphics related (no idea if related to bug 1315979)
>
> Johnathan, could you take a look, thanks!
Can you cc me on bug 1315979, if there might be a connection there? I can't access that bug...
Flags: needinfo?(jfkthame) → needinfo?(cbook)
Reporter | ||
Comment 2•8 years ago
|
||
tested http://www.espn.com/nfl/scoreboard on a linux trunk asan build from taskcluster and seems that page hung the browser after awhile
Flags: needinfo?(cbook)
Reporter | ||
Comment 3•8 years ago
|
||
i was able to reproduce this also on http://www.thedailymeal.com/recipes/rustic-cheddar-potato-soup-recipe
0x7f4de5ada0a8 is located 56 bytes to the left of global variable 'gPairConservative' defined in '/home/worker/workspace/build/src/intl/lwbrk/nsJISx4051LineBreaker.cpp:280:23' (0x7f4de5ada0e0) of size 24
0x7f4de5ada0a8 is located 0 bytes to the right of global variable 'sUnicodeLineBreakToClass' defined in '/home/worker/workspace/build/src/intl/lwbrk/nsJISx4051LineBreaker.cpp:510:23' (0x7f4de5ada080) of size 40
SUMMARY: AddressSanitizer: global-buffer-overflow (/home/tomcat/asan/opt/firefox/libxul.so+0x1f9aa4e)
Shadow bytes around the buggy address:
0x0fea3cb533c0: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00
0x0fea3cb533d0: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
0x0fea3cb533e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fea3cb533f0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
0x0fea3cb53400: 00 00 00 00 f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9
=>0x0fea3cb53410: 00 00 00 00 00[f9]f9 f9 f9 f9 f9 f9 00 00 00 f9
0x0fea3cb53420: f9 f9 f9 f9 00 00 00 f9 f9 f9 f9 f9 00 00 00 00
0x0fea3cb53430: 06 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
0x0fea3cb53440: 05 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
0x0fea3cb53450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fea3cb53460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==2227==ABORTING
Flags: needinfo?(jfkthame)
Updated•8 years ago
|
Component: Graphics → Layout: Text
Assignee | ||
Comment 4•8 years ago
|
||
I suspect this may be a regression from bug 1265631, though I haven't spotted what's wrong with the code there yet...
Assignee | ||
Comment 5•8 years ago
|
||
Ah, got it. This is triggered by bug 1299615: the new ICU (and Unicode) version has additional ULineBreak values that are not covered by the sUnicodeLineBreakToClass array.
So we need to extend that array; and we should also add some kind of assertion to detect issues like this in future.
Updated•8 years ago
|
status-firefox51:
--- → unaffected
status-firefox52:
--- → affected
Updated•8 years ago
|
status-firefox50:
--- → unaffected
status-firefox-esr45:
--- → unaffected
Reporter | ||
Comment 6•8 years ago
|
||
[Tracking Requested - why for this release]:
regression - bughunter found
tracking-firefox52:
--- → ?
Assignee | ||
Comment 7•8 years ago
|
||
A minimal testcase that will trigger this:
data:text/html,&%23x270c;
...because U+270C now gets ULineBreak value U_LB_E_BASE, which is new in ICU58 and runs off the end of the sUnicodeLineBreakToClass array that was based on ICU56 property values.
Patch coming...
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8808999 -
Flags: review?(masayuki)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment 9•8 years ago
|
||
Comment on attachment 8808999 [details] [diff] [review]
Update line-break class mapping in nsJISx4051LineBreaker to handle new classes in ICU58/Unicode 9, and add assertions to detect any future additions that will require further updates
>+ /* ZWJ = 42, [ZWJ]*/ CLASS_CHARACTER
Hmm, could be CLASS_NON_BREAKABLE but I'm not sure.
>- return sUnicodeLineBreakToClass[mozilla::unicode::GetLineBreakClass(u)];
>+ static_assert(U_LB_COUNT == mozilla::ArrayLength(sUnicodeLineBreakToClass),
>+ "Gecko vs ICU LineBreak class mismatch");
>+
>+ auto cls = mozilla::unicode::GetLineBreakClass(u);
>+ MOZ_ASSERT(cls < mozilla::ArrayLength(sUnicodeLineBreakToClass));
>+ return sUnicodeLineBreakToClass[cls];
Nice!
Attachment #8808999 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #9)
> Comment on attachment 8808999 [details] [diff] [review]
> Update line-break class mapping in nsJISx4051LineBreaker to handle new
> classes in ICU58/Unicode 9, and add assertions to detect any future
> additions that will require further updates
>
> >+ /* ZWJ = 42, [ZWJ]*/ CLASS_CHARACTER
>
> Hmm, could be CLASS_NON_BREAKABLE but I'm not sure.
I believe this is currently handled by the custom tables above anyhow, so it doesn't actually get here, but this is the value GetClass currently returns for ZWJ.
(I suspect maybe we could streamline the earlier part of GetClass and rely more on the Unicode properties, but that should be considered separately; here, I just want to fix the immediate bug.)
Assignee | ||
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c17531742f635bcb7bac22ba9e80c23afd7d49b5
Bug 1315986 - Update line-break class mapping in nsJISx4051LineBreaker to handle new classes in ICU58/Unicode 9, and add assertions to detect any future additions that will require further updates. r=masayuki
Assignee | ||
Comment 12•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/15f3c054b6a4b976359cbb741604aa57d812a9cd
Bug 1315986 followup - The static_assert to check Gecko vs ICU versions is only relevant when ENABLE_INTL_API is true. Bustage fix for Android on a CLOSED TREE.
Updated•8 years ago
|
Group: core-security → layout-core-security
Reporter | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c17531742f63
https://hg.mozilla.org/mozilla-central/rev/15f3c054b6a4
thanks Jonathan!
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•8 years ago
|
Group: layout-core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
Updated•8 years ago
|
Version: unspecified → 52 Branch
Updated•5 years ago
|
Blocks: asan-maintenance
You need to log in
before you can comment on or make changes to this bug.
Description
•