Last Comment Bug 606997 - (CVE-2011-0064) Crash with document.write marquee loop (JS:ShellCode-FG Exploit)
(CVE-2011-0064)
: Crash with document.write marquee loop (JS:ShellCode-FG Exploit)
Status: RESOLVED WORKSFORME
embargo until 2011-03-01[sg:vector-cr...
: crash, testcase
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Linux
: -- critical (vote)
: ---
Assigned To: Karl Tomlinson (:karlt)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-25 10:13 PDT by Konrad
Modified: 2014-07-28 17:47 PDT (History)
21 users (show)
rforbes: sec‑bounty+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+
-
wontfix
-
wontfix


Attachments
testcase (crashes Minefield) (1.30 KB, text/html)
2010-10-25 11:37 PDT, Brandon Sterne (:bsterne)
no flags Details
Stack with libpango symbols (8.20 KB, text/plain)
2010-10-26 13:54 PDT, Brandon Sterne (:bsterne)
no flags Details
complete stacktrace with symbols (117.19 KB, text/plain)
2010-10-26 14:04 PDT, Brandon Sterne (:bsterne)
no flags Details
de-obfuscated testcase (causes hang/OOM/crash) (239 bytes, text/html)
2010-11-09 13:30 PST, Jesse Ruderman
no flags Details
Make realloc abort in situations where failure might lead to this bug (665 bytes, patch)
2010-11-30 15:07 PST, Karl Tomlinson (:karlt)
cjones.bugs: review+
benjamin: superreview+
Details | Diff | Splinter Review
patch for Pango (6.14 KB, patch)
2010-12-01 19:24 PST, Karl Tomlinson (:karlt)
mozilla: review-
Details | Diff | Splinter Review

Description Konrad 2010-10-25 10:13:34 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.2.11) Gecko/20101012 Firefox/3.6.11 GTB7.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.2.11) Gecko/20101012 Firefox/3.6.11 GTB7.1

I'm from Poland
I do not know English.
I'll write in Polish.

Old version  not by me, detect antywius JS:ShellCode-FG [Exploit]
http://pastebin.com/Dpd56jvw

My vwerion RIP not detect antivirus 
http://pastebin.com/YrhzMCEG

Bawiłem się kodami vbs malware i wykorzystałem do maskowania kodu.
Gdyby nie zachęta w postaci nagrody to bym nie napisał jak by nie było to krytyczny błąd, js za dużo może od strony użytkownika niestety.
Ale lubię programować jako tako więc postanowiłem się podzielić. 
Jedyna odporna przeglądarka która znam to google chrome.
Osobiście wole Mozillę za szereg wtyczek które ubóstwiam.



Reproducible: Always
Comment 1 Konrad 2010-10-25 10:57:19 PDT
(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.2.11)
> Gecko/20101012 Firefox/3.6.11 GTB7.1
> Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.2.11)
> Gecko/20101012 Firefox/3.6.11 GTB7.1
> 
> I'm from Poland
> I do not know English.
> I'll write in Polish.
> 
> Old version  not by me, detect antywius JS:ShellCode-FG [Exploit]
> http://pastebin.com/Dpd56jvw
> 
> My vwerion RIP not detect antivirus 
> http://pastebin.com/YrhzMCEG
> 
> Bawiłem się kodami vbs malware i wykorzystałem do maskowania kodu vbs hex.
> Gdyby nie zachęta w postaci nagrody to bym nie napisał jak by nie było to
> krytyczny błąd, js za dużo może od strony użytkownika niestety.
> Ale lubię programować jako tako więc postanowiłem się podzielić. 
> Jedyna odporna przeglądarka która znam to google chrome.
> Osobiście wole Mozillę za szereg wtyczek które ubóstwiam.
> 
> 
> 
> Reproducible: Always
Comment 2 Brandon Sterne (:bsterne) 2010-10-25 11:37:32 PDT
Created attachment 485793 [details]
testcase (crashes Minefield)
Comment 3 Brandon Sterne (:bsterne) 2010-10-25 11:54:44 PDT
bp-754af2b3-c195-4424-ae20-96a3e2101025
Comment 5 chris hofmann 2010-10-26 13:25:46 PDT
google translation of original report

> I played within vbs malware, and I used to mask the vbs hex code.
> If it was not an incentive in the form of prizes I would have written as if it was not
> Fatal error js too much from the user may unfortunately.
> But I like programming a fashion so I decided to share.
> The only browser that I know immune to google chrome.
> Personally, I prefer Mozilla for a number of plugins which adore.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-10-26 13:28:31 PDT
Karl, can you look into this?
Comment 7 Brandon Sterne (:bsterne) 2010-10-26 13:54:48 PDT
Created attachment 486164 [details]
Stack with libpango symbols
Comment 8 Brandon Sterne (:bsterne) 2010-10-26 14:04:03 PDT
Created attachment 486165 [details]
complete stacktrace with symbols
Comment 9 Karl Tomlinson (:karlt) 2010-10-26 15:02:13 PDT
I'll look into the Pango code.
Has the testcase been tried on other platforms?
Comment 10 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-11-02 13:20:56 PDT
Definitely looks like a Pango bug. Probably needs to go upstream.
Comment 11 Behdad Esfahbod 2010-11-02 17:01:50 PDT
Looks like OOM.  Do you think malloc() may return NULL for a 200mb request?  Pango doesn't deal with OOM, though next version using external harfbuzz will do a fair job.  Can you make sure you don't pass on strings 8388607 characters long to pango?
Comment 12 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2010-11-02 17:17:31 PDT
That's only 8-16MB of text. We should be able to handle that. Perhaps we could break up the string into pieces, like we do for shapers on other platforms, but that feels like working around a bug.

Shouldn't OOM trigger a clean abort with an obvious stack trace rather than dereferencing NULL or whatever's happening here?
Comment 13 Behdad Esfahbod 2010-11-02 17:19:59 PDT
(In reply to comment #12)
> That's only 8-16MB of text. We should be able to handle that. Perhaps we could
> break up the string into pieces, like we do for shapers on other platforms, but
> that feels like working around a bug.
> 
> Shouldn't OOM trigger a clean abort with an obvious stack trace rather than
> dereferencing NULL or whatever's happening here?

Donno if realloc aborts or returns NULL these days.  I can try making it try to deal with it.  Let me cook a patch tonight and see.
Comment 14 Daniel Veditz [:dveditz] 2010-11-03 01:51:11 PDT
So far on Windows XP (testing 3.6.x) I've only seen unexploitable crashes like operator new throwing on OOM.
Comment 15 Karl Tomlinson (:karlt) 2010-11-03 16:14:01 PDT
(In reply to comment #11)
> Looks like OOM.  Can you make sure you don't pass on strings 8388607 characters
> long to pango?

Firefox here (x86_64) seems to have a writable segment at 0x0060d000 (6344704).
If we break up the string, we can choose where to break it up.
How do we choose the number?
Comment 16 Behdad Esfahbod 2010-11-04 16:00:25 PDT
(In reply to comment #13)
> Let me cook a patch tonight and see.

To be realistic, I don't think I get to do it anytime soon.  One can try to backport the buffer malloc failure handling from hb-ng to pango, but it's not a high priority for me right now.


(In reply to comment #15)
> (In reply to comment #11)
> > Looks like OOM.  Can you make sure you don't pass on strings 8388607 characters
> > long to pango?
> 
> Firefox here (x86_64) seems to have a writable segment at 0x0060d000 (6344704).

Humm?


> If we break up the string, we can choose where to break it up.
> How do we choose the number?

I'd say, 100,000 chars?  I have a hard time imagining a legitimate usecase for a 100,000 chars long paragraph.
Comment 17 Karl Tomlinson (:karlt) 2010-11-04 16:12:46 PDT
> (In reply to comment #15)
> > (In reply to comment #11)
> > > Looks like OOM.  Can you make sure you don't pass on strings 8388607 characters
> > > long to pango?
> > 
> > Firefox here (x86_64) seems to have a writable segment at 0x0060d000 (6344704).
> 
> Humm?

My comment was on the understanding that limiting the size of the offset from NULL to the portion of the address space that can't be modified would keep the outcome to a DoS.  And I'm curious how much space we have at the beginning of the address space that can't be modified.

Though a smaller limit would also help make the OOM a little less likely, I guess.  Being conservative is fine, but I'm still curious re the size of non-writeable address space, so that I know we're being conservative.

We can limit the number of characters.  Is there any limit on the number of glyphs per character?
Comment 18 Behdad Esfahbod 2010-11-04 16:18:15 PDT
(In reply to comment #17)
> We can limit the number of characters.  Is there any limit on the number of
> glyphs per character?

Not really.  With webfonts you can exponentially expand the buffer length with a tiny tiny font.  I'm actually working on such test fonts right now.
Comment 19 Jesse Ruderman 2010-11-09 13:30:14 PST
Created attachment 489278 [details]
de-obfuscated testcase (causes hang/OOM/crash)
Comment 20 Karl Tomlinson (:karlt) 2010-11-09 13:41:02 PST
Jonathan, does bug 527276 provide us with any limit on the number of glyphs per character?
Comment 21 Behdad Esfahbod 2010-11-09 13:43:09 PST
(In reply to comment #20)
> Jonathan, does bug 527276 provide us with any limit on the number of glyphs per
> character?

Don't think so.  Large expansion can come from legitimate GSUB tables.  And the OT sanitizer doesn't even support GSUB.
Comment 22 Zibi Braniecki [:gandalf][:zibi] 2010-11-09 13:55:18 PST
Konrad: Dziękujemy za zgłoszenie. :)
Grupa d/s bezpieczeństwa analizuje je teraz i jak tylko uzyskamy wyniki skontaktujemy się z Tobą mailowo lub w tym błędzie.

Can we switch status to confirmed? Do we have enough data to weight the security risk here?
Comment 23 Jonathan Kew (:jfkthame) 2010-11-09 13:59:16 PST
(In reply to comment #20)
> Jonathan, does bug 527276 provide us with any limit on the number of glyphs per
> character?

No; OTS does not (currently) even attempt to check/sanitize OpenType layout tables (in particular, GSUB, which is the one that could be used to generate large numbers of glyphs from a short string).

Having said that, the typical DoS (such as this one) does not directly involve generating huge numbers of *glyphs* but rather inserting very long *strings* into the document. This stresses the shaping process because we shape the entire string at once.

There's a patch in bug 606714 that addresses this by splitting very large strings (preferring whitespace, or at least cluster boundaries) so that we shape them in sections (about 32K chars). This is implemented in gfxFont::InitTextRun, so potentially applies to any back-end, but currently some platforms may override this method and so they'd need to handle it separately; I haven't checked the Pango case specifically.

(In reply to comment #12)
> Shouldn't OOM trigger a clean abort with an obvious stack trace rather than
> dereferencing NULL or whatever's happening here?

Yes, OOM ought to be handled properly/safely, and if it's not, we ought to fix that. But we should also try to avoid passing multi-megabyte strings to shapers, as the shaping process necessarily involves buffer allocations of several times the size of the input text, and there's no realistic use-case that actually requires shaping such strings in a single operation.
Comment 24 Karl Tomlinson (:karlt) 2010-11-09 14:17:07 PST
Confirming based on comment 11.

realloc() is in our code (for apps built with the default m-c configure.in at least), so I think the simplest protection from fonts with large numbers of glyphs per characters is to make realloc abort on failure.

(realloc is what is involved here and malloc isn't quite the same risk, because often malloced data is first written near the beginning of the segment leading to a safe crash.)

(In reply to comment #23)
> Having said that, the typical DoS (such as this one) does not directly involve
> generating huge numbers of *glyphs* but rather inserting very long *strings*
> into the document. This stresses the shaping process because we shape the
> entire string at once.
> 
> There's a patch in bug 606714 that addresses this by splitting very large
> strings (preferring whitespace, or at least cluster boundaries) so that we
> shape them in sections (about 32K chars). This is implemented in
> gfxFont::InitTextRun, so potentially applies to any back-end, but currently
> some platforms may override this method and so they'd need to handle it
> separately; I haven't checked the Pango case specifically.

Yes, that path won't be used even after bug 597147 lands.
Comment 25 Daniel Veditz [:dveditz] 2010-11-09 17:01:20 PST
Using FF 3.6.13pre on Windows (where originally reported) I only get crashes due to operator new throwing on OOM. We did fix bug 607222 in Firefox 3.6.12 and this bug was filed against 3.6.11--could that be a fix? Although both testcases use document.write the other one also required insertions.
Comment 26 Daniel Veditz [:dveditz] 2010-11-09 18:22:40 PST
No, I'm seeing exactly the same thing on Windows XP with 3.6.11 as originally reported. On Windows this is a sg:dos OOM bug, and basically a duplicate of several similar testcases.

Is there a worse-than-DoS problem on Linux with pango?
Comment 27 Karl Tomlinson (:karlt) 2010-11-09 18:31:27 PST
On Linux, with Pango, when realloc fails on OOM, memory is then overwritten at an offset from NULL equal to the length the end of the old buffer, which could be arbitrary length depending on memory limits.  That seems worse than DoS to me.

http://git.gnome.org/browse/pango/tree/pango/opentype/hb-buffer.c?id=1e53d4d5904445c740a374ea8492935f95bf1654#n163
Comment 28 Daniel Veditz [:dveditz] 2010-11-16 13:09:57 PST
Agreed, we need to fix (or work around) that. Changing platform for Linux, the OOM crashes on the original windows aren't as serious.
Comment 29 Karl Tomlinson (:karlt) 2010-11-23 13:17:15 PST
In Linux default configurations, only one page (4k) is protected from being
mapped into user address space (DEFAULT_MMAP_MIN_ADDR).
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=mm/Kconfig;h=c2c8a4a11898f949faa72cf5702d8c21e64f78b9;hb=HEAD#l240

AFAIK, the logic in arch_get_unmapped_area_topdown means that the next page could be used in low memory situations.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=mm/mmap.c;h=b179abb1474ae41bff47060b5241045d3b1b12ad;hb=HEAD#l1525

(BTW, it seems that the bottomup logic will not necessarily kick in if
mmap_min_addr > one page, because an unmapped area can be selected below
mmap_min_addr.  mmap_min_addr gets checked after the area is selected.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=mm/mmap.c;h=b179abb1474ae41bff47060b5241045d3b1b12ad;hb=HEAD#l1096)
Comment 30 Karl Tomlinson (:karlt) 2010-11-23 13:18:11 PST
sizeof(hb_internal_glyph_info_t) = 20 (on 32 and 64 bit machines)

That gives us a safe limit of 4k/20 which we could round up to 205 glyphs.
Given the "new_allocated += (new_allocated >> 1) + 8" pattern and possible
initial values of 32 or 64, we could expect a safe SEGV for lengths of up to 227 glyphs.
Comment 31 Johnny Stenback (:jst, jst@mozilla.com) 2010-11-30 13:50:51 PST
Marking this a betaN blocker for now. Is this something that we're likely to fix in code we ship, or are we dependent on os distros here for an updated pango library? If we ship it ourselves, we IMO need this in a beta to get decent testing on it etc.
Comment 32 Karl Tomlinson (:karlt) 2010-11-30 14:54:50 PST
If we stripped GSUB tables from downloaded fonts (which would be unfortunate),
IIUC the number of glyphs in a run would be bounded by k0 + k1 * c, where c
is the number of characters, and we could control c, but the limit is
uncomfortably small, making this approach unappealing.
(I'd need to study the code much more carefully to calculate the constants k0 and k1, and it's complex code, so I'm not super confident about getting the numbers right.)
Comment 33 Karl Tomlinson (:karlt) 2010-11-30 14:55:41 PST
I think that leaves two options:

1) Write a patch to add realloc failure checking to Pango, and depend on OS
   vendors to pick up the patch.

2) Make realloc abort in situations where failure might lead to this bug.
Comment 34 Karl Tomlinson (:karlt) 2010-11-30 15:07:57 PST
Created attachment 494190 [details] [diff] [review]
Make realloc abort in situations where failure might lead to this bug

This aims to make realloc fail in the minimum set of situations and still avoid this bug.
Pango always starts writing at the next byte in the newly allocated buffer, so we get a safe SEGV if the length of the previous buffer is known to be < 4096.
Comment 35 Karl Tomlinson (:karlt) 2010-11-30 15:09:12 PST
CC'ing some people that I know have thoughts on infallible malloc.

Note that GLib and GObject have allocators that abort on failure (and these
libraries are used by Pango), so abort on OOM is already likely.  However, the
proposed patch here increases the number of situations that lead to abort on
OOM.  A don't know how to comment on how significant that increase is.
Comment 36 Karl Tomlinson (:karlt) 2010-11-30 15:16:30 PST
I guess we should wait for the outcome of this meeting:
https://wiki.mozilla.org/Platform/InfallibleMalloc
Comment 37 Karl Tomlinson (:karlt) 2010-11-30 17:13:13 PST
The crash in comment 8 can be reproduced with attachment 489278 [details] and an x86_64 m-c debug build with an address space limit of 2150 MB.

The requested size from realloc there is 58 MB.
Reducing the address space limit a touch to 2100 MB instead results in an abort during an attempted infallible allocation of 64 MB here:
http://hg.mozilla.org/mozilla-central/annotate/60d9203fdc35/parser/html/nsHtml5TreeBuilderCppSupplement.h#l312

(and other address space limits lead to other infallible allocation failures.)
Comment 38 Boris Zbarsky [:bz] 2010-11-30 19:17:44 PST
We shouldn't be doing potentially-64MB allocations as infallible allocations!  Can we please get a bug filed on the HTML5 parser for that bit from comment 37?

The pango realloc is harder...  We should definitely wait on the outcome of tomorrow's meeting (and in particular, maybe we can have an infallible realloc even if malloc stays fallible...).

Did the earlier suggestions to limit the string lengths we pass to pango not pan out?
Comment 39 Karl Tomlinson (:karlt) 2010-11-30 19:34:51 PST
(In reply to comment #38)
> Did the earlier suggestions to limit the string lengths we pass to pango not
> pan out?

As part of that approach, we'd need to keep lengths to something like 50-100 characters.  (I don't know exactly how many.)  I'd need to measure how that affected peformance and it would also sometimes prevent ligatures from forming.

As the other part, we'd need to incapacitate downloaded fonts so that they didn't produce many glyphs per character.  That's a particularly big issue on branches where we don't have HarfBuzz for any scripts.
Comment 40 Karl Tomlinson (:karlt) 2010-11-30 19:38:03 PST
A very ugly, but doable approach could be to make realloc infallible only during calls to pango_shape.
Comment 41 Boris Zbarsky [:bz] 2010-11-30 20:04:32 PST
We can't really make realloc generally infallible on those branches either, right?
Comment 42 Behdad Esfahbod 2010-11-30 20:27:48 PST
Seeing how much trouble this bug is making, if a pango patch that is then left to distributions to pick up would "fix" this bug, I can be bothered to make that.
Comment 43 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-12-01 11:18:26 PST
(In reply to comment #30)
> sizeof(hb_internal_glyph_info_t) = 20 (on 32 and 64 bit machines)
> 
> That gives us a safe limit of 4k/20 which we could round up to 205 glyphs.
> Given the "new_allocated += (new_allocated >> 1) + 8" pattern and possible
> initial values of 32 or 64, we could expect a safe SEGV for lengths of up to
> 227 glyphs.

On my x86-64 machine, firefox seems to be consistently loaded with the bottom 4MB of address space unmapped (and my kernel's configured mmap_min_addr is 65536).  Certainly this value would be different on x86 and ARM, but I wonder if we could attempt to map K bytes in the bottom of our address space with no access.  Then we could choose the max-glyph constant based on the amount of no-access address space we expect to map, instead of this very small unmappable limit.
Comment 44 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-12-01 11:30:19 PST
In an x86 VM of mine, dsos are the mapped at the lowest addresses (with ASLR too).  But, on several trials I didn't see libs mapped below 1D8000 (1933312).
Comment 45 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-12-01 11:38:13 PST
Tried a few more times with fennec-bin in the VM: the lowest mapped address was always 00110000 (1114112).
Comment 46 Karl Tomlinson (:karlt) 2010-12-01 11:40:07 PST
(In reply to comment #43)
> Certainly this value would be different on x86 and ARM, but I wonder
> if we could attempt to map K bytes in the bottom of our address space with no
> access.

That sounds like an excellent idea in general on x86_64 systems, where there is plenty of address space.

On 32-bit systems, address space is in short supply so it gets harder to pick a reasonable K.
Comment 47 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2010-12-01 12:10:39 PST
Comment on attachment 494190 [details] [diff] [review]
Make realloc abort in situations where failure might lead to this bug

It appears that we only use pango with gtk2 and qt, which in practice implies running on the linux kernel.  It seems like this patch assumes overcommit and so rare realloc failures that are visible to jemalloc.  Otherwise, this would essentially make ns*Array/ns*String infallible with no fallible recourse.  That logic seems fine to me.
Comment 48 Karl Tomlinson (:karlt) 2010-12-01 19:24:03 PST
Created attachment 494625 [details] [diff] [review]
patch for Pango

(In reply to comment #42)
> Seeing how much trouble this bug is making, if a pango patch that is then left
> to distributions to pick up would "fix" this bug, I can be bothered to make
> that.

Thanks, Behdad.  Yes, I think we should fix this in Pango.  (We're not
confident about the consequences from possible workarounds in Gecko.)

This is adapted from your HarfBuzz patch
http://cgit.freedesktop.org/harfbuzz/commit/?id=a6a79df5fe2e

The only functional change is that positions is now always allocated in
hb_buffer_ensure.  Whether that makes any difference in practice, I don't
know.

With this patch, I get a safe abort here (as expected):
#6  0x00007f7f6be4258d in *__GI_abort () from /lib/libc.so.6
#7  0x00007f7f6cea78a2 in g_logv (log_domain=<value optimized out>, 
    log_level=<value optimized out>, format=<value optimized out>, 
    args1=0x7fff53864cc0) at gmessages.c:557
#8  0x00007f7f6cea7946 in g_log (
    log_domain=0x7da7 <Address 0x7da7 out of bounds>, log_level=32167, 
    format=0x6 <Address 0x6 out of bounds>) at gmessages.c:577
#9  0x00007f7f6cea5ac4 in g_realloc (mem=<value optimized out>, 
    n_bytes=83886080) at gmem.c:238
#10 0x00007f7f6dcf2454 in pango_glyph_string_set_size (string=0x7f7f34f35040, 
    new_len=4194297) at glyphstring.c:89
#11 0x00007f7f4331348c in basic_engine_shape (engine=<value optimized out>, 
    font=0x7f7f41d28f50, 
    text=0x7f7f02200008 " \\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x4"..., length=<value optimized out>, 
    analysis=0x7fff53865030, glyphs=<value optimized out>) at basic-fc.c:155
#12 0x00007f7f6dd09a5a in pango_shape (
    text=0x7f7f02200008 " \\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x4"..., length=4194297, 
    analysis=0x7fff53865030, glyphs=0x7f7f34f35040) at shape.c:55
#13 0x00007f7f724caedc in InitGlyphRunWithPangoAnalysis (
    aTextRun=0x7f7f17de6aa0, 
    aUTF8=0x7f7f02200008 " \\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x42\\x4"..., aUTF8Length=4194297, 
    aUTF16Offset=0x7fff5386507c, aAnalysis=0x7fff53865030, 
    aOverrideSpaceWidth=256000)
    at /home/karl/moz/dev/gfx/thebes/gfxPangoFonts.cpp:2874
Comment 49 Henri Sivonen (:hsivonen) (Not doing reviews or reading bugmail until 2016-08-01) 2010-12-02 00:50:32 PST
(In reply to comment #38)
> Can we please get a bug filed on the HTML5 parser for that bit from comment 37?

Filed bug 616094. (I didn't nominate it as a blocker, because a previously-filed analogous bug was delayed to wait for the infallible malloc machinery itself learning to stop active parsers.)
Comment 50 Behdad Esfahbod 2010-12-02 05:13:35 PST
Thanks Karl.  LGTM.
Comment 51 Jonathan Kew (:jfkthame) 2011-01-08 07:10:08 PST
FWIW, the patch in bug 617905 should mean that it is significantly more difficult to hit this issue now, as it prevents huge strings being passed to Pango for shaping in a single operation; they are broken up and shaped in sections. This will help to limit Pango's need to allocate huge working buffers.

It would still be possible to hit this, I think, but crafting an example that pushes us close enough to OOM that allocation will fail within Pango while shaping a 32K-character run (yet does not simply hit OOM elsewhere and crash safely) will be a lot more difficult and fairly specific to the exact system configuration in use.
Comment 52 chris hofmann 2011-01-10 19:53:11 PST
Konrad, please contact me at chofmann@mozilla.com if you are interested in a bug bounty for your help on this bug.
Comment 53 Zibi Braniecki [:gandalf][:zibi] 2011-01-11 02:39:45 PST
I'll contact the reporter directly in polish as well.
Comment 54 Joe Drew (not getting mail) 2011-01-23 15:52:55 PST
Jonathan/Karl, given comment 51, it looks like this doesn't need to be a 2.0 hardblocker any more. 

Chris, is there any reason we shouldn't take Karl's realloc patch, attachment 494190 [details] [diff] [review], on 2.0, 1.9.2, or 1.9.1?

Behdad, can you look into reviewing Karl's patch to pango, attachment 494625 [details] [diff] [review]? Who should look into getting that upstream so distros can pick up the patch?
Comment 55 Behdad Esfahbod 2011-01-27 09:05:03 PST
The pango patch looks good.

It's probably best to report this to vendorsec.
Comment 56 :Ehsan Akhgari 2011-02-09 22:08:31 PST
Comment 54 says that this doesn't need to be a blocker any more, but it only moves it to be a soft-betaN-blocker.  If we let this remain in its current status, it will most likely miss 2.0.  I'm not sure if we need to do anything on our side here anyway, but I'm nominating it for retriage anyways.
Comment 57 chris hofmann 2011-02-10 00:53:22 PST
(In reply to comment #54)
> Jonathan/Karl, given comment 51, it looks like this doesn't need to be a 2.0
> hardblocker any more. 
> 
> Chris, is there any reason we shouldn't take Karl's realloc patch, attachment
> 494190 [details], on 2.0, 1.9.2, or 1.9.1?
> 

This is probably a better question for dveditz, bsterne, or someone closer to managing the 1.9.x releases.

If its feasible for researchers to find a way to develop and exploit around the second comment in comment 51 I think we should assume some higher risk for needing to get this in to 2.0, since independent research turned up the original problem.
Comment 58 Johnny Stenback (:jst, jst@mozilla.com) 2011-02-10 14:34:15 PST
Joe, Karl, seems we should take either of the two ramining patches for this sg:critical bug for 2.0. Can you guys make that happen?
Comment 59 Karl Tomlinson (:karlt) 2011-02-10 15:18:22 PST
I can't push the Pango patch.

Behdad did you think this should be reported to OS vendor security teams before pushing the patch?
Comment 60 Karl Tomlinson (:karlt) 2011-02-10 15:22:36 PST
Comment on attachment 494190 [details] [diff] [review]
Make realloc abort in situations where failure might lead to this bug

If this is too scary, my best idea is to add a hook to jemalloc so that this can be turned on and off during the risky Pango call.
Comment 61 Behdad Esfahbod 2011-02-10 20:26:00 PST
(In reply to comment #59)
> I can't push the Pango patch.
> 
> Behdad did you think this should be reported to OS vendor security teams before
> pushing the patch?

That's what those guys typically want to see, an embargo release date before anything is pushed out publicly.
Comment 62 Karl Tomlinson (:karlt) 2011-02-11 14:43:30 PST
Do we have anyone who knows the appropriate way to notify the appropriate set of OS vendors?  That sounds like the best way forward here.

If we were to land the patch here, we should probably make sure that any other browser vendors that use Pango have some notice.
Comment 63 Behdad Esfahbod 2011-02-13 10:30:17 PST
vendor-sec@lst.de
Comment 64 Karl Tomlinson (:karlt) 2011-02-14 21:40:28 PST
I'm not a subscriber to vendor-sec and I probably shouldn't be.
(I'm hoping it is not that easy to join the list.)

Does Mozilla have any subscribers that would be willing to submit attachment 494625 [details] [diff] [review] and watch for response.

If not, I can send off of the patch.  I won't know what discussion takes place, but I can ask for a reply.
Comment 65 Behdad Esfahbod 2011-02-15 11:23:49 PST
As far as I know, you don't need to subscribe to vendor-sec (nor can you easily).  You just email them with details of the issue and they reply...
Comment 66 Daniel Veditz [:dveditz] 2011-02-15 11:36:50 PST
Assigning CVE-2011-0064 to the Pango bug, and I'll send that with the patch.
Comment 67 Johnathan Nightingale [:johnath] 2011-02-17 12:15:20 PST
Hey Dan, do we have an ETA for closing this in our own code?
Comment 68 Daniel Veditz [:dveditz] 2011-02-17 23:12:38 PST
I did not get any response from vendor-sec, guess I'll ping the distro guys on our own security group directly.

Is there anything to close in our own code? I'm confused about the status of attachment 494190 [details] [diff] [review] -- it's reviewed and not obsolete, but Karl seemed to think it was worth avoiding in comment 48.

(In reply to comment #57)
> (In reply to comment #54)
> > Chris, is there any reason we shouldn't take Karl's realloc patch, attachment
> > 494190 [details], on 2.0, 1.9.2, or 1.9.1?
> 
> This is probably a better question for dveditz, bsterne, or someone closer to
> managing the 1.9.x releases.

I think that question was addressed to Chris Jones, not Hofmann. I think the answer to that question is also the answer to Johnath's question.
Comment 69 Daniel Veditz [:dveditz] 2011-02-18 13:03:49 PST
Canonical replied

> Unless anyone objects, how about a CRD of Tuesday, 2011-03-01?

and Red Hat agreed. Haven't heard from other vendors but there haven't been any objections either.
Comment 70 Karl Tomlinson (:karlt) 2011-02-20 19:45:17 PST
Attachment 494190 [details] [diff] was a workaround.  It is unnecessary and there is nothing that we need to do in our code if we can depend on distributions to apply attachment 494625 [details] [diff] [review] to Pango (CVE-2011-0064).

Attachment 494190 [details] [diff] was really a last resort as it comes with the risk of making the application abort more often in low memory situations.  I expect there is a non-negligible number of situations where that will happen, but those may (or may not) be situations where we were about to abort, or at least badly malfunction, anyway.
Comment 71 Daniel Veditz [:dveditz] 2011-02-23 10:12:36 PST
Should we close the bug then, or is it worth doing something about the OOM crash on other platforms?
Comment 72 Karl Tomlinson (:karlt) 2011-02-23 18:55:49 PST
Making the browser resilient to OOM DoS attacks is a big project beyond the scope of this bug IMO.

I'll tentatively mark this WORKSFORME in the sense that it is fixed when the Pango patch is applied.

Note You need to log in before you can comment on or make changes to this bug.