1184009 - (CVE-2015-4491) gdk-pixbuf heap overflow and DoS affecting Firefox

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Gustavo Grieco]

Attachment: overflow-firefox.bmp.gz

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0
Build ID: 20150629114848

Steps to reproduce:

I'm forwarding this bug because it clearly affects last versions of Firefox. This was reported to upstream here (https://bugzilla.gnome.org/show_bug.cgi?id=752297) and it is supposed to be fixed (i haven't tested the patch yet).
Requirements:

* Linux x86_64
* gdk-pixbuf 2.31 or newer (you can upgrade only this package in Ubuntu/Debian download and installing from here: https://packages.debian.org/sid/libgdk-pixbuf2.0-0 or http://packages.ubuntu.com/vivid/libgdk-pixbuf2.0-0). Older versions are more difficult to exploit, but the issue is still there.
* At least 10GB of real/virtual memory (a big swapfile will do the trick if you don't have enough physical memory).

Steps to reproduce:

1. Download and decompress firefox-overflow.bmp.gz  (it is compressed to avoid crashing my own browser!)
2. Try to open (ctrl+O) or attach firefox-overflow.bmp
3. Boom


Actual results:

Firefox crashes inside gdk-pixbuf code. Details:

Program received signal SIGSEGV, Segmentation fault.
make_filter_table (filter=0x7ffffffead10) at pixops.c:1294
1294	pixops.c: No such file or directory.
(gdb) info registers 
rax            0x0	0
rbx            0x2	2
rcx            0x0	0
rdx            0x0	0
rsi            0x7fffbae00000	140736328630272
rdi            0x7fffbcfc8d00	140736364055808
rbp            0x7ffffffead10	0x7ffffffead10
rsp            0x7ffffffeab20	0x7ffffffeab20
r8             0x2b0000	2818048
r9             0x0	0
r10            0x4000010	67108880
r11            0x7fffbeb5000c	140736392921100
r12            0x8	8
r13            0x1	1
r14            0x1000003	16777219
r15            0x7fffbc436c00	140736351923200
rip            0x7fffedf57239	0x7fffedf57239 <pixops_process+681>
eflags         0x10246	[ PF ZF IF RF ]
cs             0x33	51
ss             0x2b	43
ds             0x0	0
es             0x0	0
fs             0x0	0
gs             0x0	0
(gdb) x/i $rip
=> 0x7fffedf57239 <pixops_process+681>:	mov    %edx,(%rsi,%rax,4)

Here %rsi depens on the dimension of the malformed bmp.


Expected results:

Of course, Firefox shouldn't so easily be exploitable. Nevertheless, it is not clear how Firefox can prevent this kind of vulnerabilities in the future (unless your re-implement gdk-pixbuf functions)

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: limit image preview sizes

This patch does some overflow checking to verify that the filter table created within gdk-pixbuf won't overflow on allocation when trying to scale the image. Due to the fact that the upstream "fix" for the issue merely makes the allocation always fail in such cases anyway, it is sufficient for us to merely do the check on our end so that we're not dependent on users having the upstream library fixed.

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
The patch doesn't explicit mention the issue with the overflow, but just does the size check, which by itself shouldn't highlight the possible exploit.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No. However, there is an upstream bug report filed on the issue, so that this issue is unfortunately known beyond our bug reporting system. I would recommend we patch this ASAP.

> Which older supported branches are affected by this flaw?
This affects branches as far back as at least ESR 31, that I can see. Pretty much anything that uses "gdk_pixbuf_new_from_file_at_size". The problem is not in our code, per se, but in vulnerabilities introduced in gdk-pixbuf, so even our code which may have previously been safe became unsafe due to the upstream issue.

> If not all supported branches, which bug introduced the flaw?
We appear to have started using this gdk-pixbuf feature in this changeset: https://hg.mozilla.org/releases/mozilla-esr31/rev/e92be8bbe5ef

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
This patch should work with older branches. At worst, it will disable previews on large images, but only in cases where crashes/overflows would have normally occurred.

> How likely is this patch to cause regressions; how much testing does it need?
This shouldn't cause any notable regressions other than as noted - in cases where a crash or exploit might otherwise be observed, instead a lack of a preview image will be observed. This change at least blocks the reported upstream vulnerability as far as my own testing indicated.

Comment 3

[Andrew Comminos [:acomminos]]

Comment on attachment 8636664 [details] [diff] [review]
limit image preview sizes

Review of attachment 8636664 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gtk/nsFilePicker.cpp
@@ +105,4 @@
>    // Only scale down images that are too big
>    if (preview_width > MAX_PREVIEW_SIZE || preview_height > MAX_PREVIEW_SIZE) {
> +    if (ceil(preview_width / double(MAX_PREVIEW_SIZE) + 1.0) *
> +          ceil(preview_height / double(MAX_PREVIEW_SIZE) + 1.0) < 0x7FFFFF) {

It's probably a good idea to add a comment briefly explaining why this is done. I'm okay with the magic number use as it's not really an intuitive quantity.

A check for affected GDK pixbuf versions using gdk_pixbuf_*_version may make sense as well here, and provide a bit of context.

Comment 4

[Lee Salzman [:lsalzman]]

(In reply to Andrew Comminos [:acomminos] from comment #3)
> Comment on attachment 8636664 [details] [diff] [review]
> limit image preview sizes
> 
> Review of attachment 8636664 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/gtk/nsFilePicker.cpp
> @@ +105,4 @@
> >    // Only scale down images that are too big
> >    if (preview_width > MAX_PREVIEW_SIZE || preview_height > MAX_PREVIEW_SIZE) {
> > +    if (ceil(preview_width / double(MAX_PREVIEW_SIZE) + 1.0) *
> > +          ceil(preview_height / double(MAX_PREVIEW_SIZE) + 1.0) < 0x7FFFFF) {
> 
> It's probably a good idea to add a comment briefly explaining why this is
> done. I'm okay with the magic number use as it's not really an intuitive
> quantity.
> 
> A check for affected GDK pixbuf versions using gdk_pixbuf_*_version may make
> sense as well here, and provide a bit of context.

I was afraid to add details like this because it gives too much information about how to exploit the issue.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

sec-approval+ for trunk. We should have branch patches for Aurora, Beta, and ESR38 created and nominated so they can go in once trunk is green.

Comment 6

[Lee Salzman [:lsalzman]]

Try results for patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7236168282db

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43e50f129af4

Comment 8

[Al Billings [:abillings - ex-MoCo]]

(In reply to Al Billings [:abillings] from comment #5)
> sec-approval+ for trunk. We should have branch patches for Aurora, Beta, and
> ESR38 created and nominated so they can go in once trunk is green.

I don't see any Aurora, Beta, or ESR38 patches here. Is there a reason they weren't made and nominated?

Comment 9

[Lee Salzman [:lsalzman]]

(In reply to Al Billings [:abillings] from comment #8)
> (In reply to Al Billings [:abillings] from comment #5)
> > sec-approval+ for trunk. We should have branch patches for Aurora, Beta, and
> > ESR38 created and nominated so they can go in once trunk is green.
> 
> I don't see any Aurora, Beta, or ESR38 patches here. Is there a reason they
> weren't made and nominated?

The patch should work for all of them.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Ok. Well, generally, you, as the assignee, need to nominate them for inclusion or they won't get approved and go in on the branches.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8636664 [details] [diff] [review]
limit image preview sizes

This has already landed on inbound. Better get it on the branches. Beta+ Aurora+ ESR38+

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/43e50f129af4
https://hg.mozilla.org/releases/mozilla-aurora/rev/e67096569716
https://hg.mozilla.org/releases/mozilla-beta/rev/9b861360b297
https://hg.mozilla.org/releases/mozilla-esr38/rev/c9d5dc392dde

Comment 13

[Ada [:adalucinet]]

While following STR via comment 0, I got stuck at installing gdk-pixbuf 2.31 toolkit; encountered various errors, including to install some circular dependency and this broke the OS (Ubuntu 14.04 x64). Since I am not able to reproduce/verify this issue here, Gustavo Grieco could you please confirm if this issue is fixed on 40.0 RC [1] and ESR 38.1.1 [2] builds? Thanks in advance!

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/40.0-candidates/build2/
[2] http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/38.1.1esr-candidates/build2/

Comment 14

[Gustavo Grieco]

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #13)
> While following STR via comment 0, I got stuck at installing gdk-pixbuf 2.31
> toolkit; encountered various errors, including to install some circular
> dependency and this broke the OS (Ubuntu 14.04 x64). 

I know, it almost happened to me.

> Since I am not able to reproduce/verify this issue here, Gustavo Grieco could you please confirm if
> this issue is fixed on 40.0 RC [1] and ESR 38.1.1 [2] builds? Thanks in
> advance!

Sure thing. I have the vulnerable packages installed at my laptop at home. I will be able to test it but later today. Is it ok?

> 
> [1]
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/40.0-candidates/
> build2/
> [2]
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/38.1.1esr-candidates/
> build2/

Comment 15

[Gustavo Grieco]

I'm affraid it is still aborting:

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007fffef212c13 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
(gdb) bt
#0  0x00007fffef212c13 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#1  0x00007fffef212d72 in g_log () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007fffef211644 in g_malloc () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007fffedf1f0ff in make_filter_table (filter=0x7ffffffead20) at pixops.c:1275
#4  pixops_process (dest_buf=0x7fffc58a3800 'Z' <repeats 200 times>..., render_x0=0, render_y0=0, render_x1=180, render_y1=1, dest_rowstride=540, 
    dest_channels=3, dest_has_alpha=0, src_buf=0x7fff38f00000 "", src_width=2097181, src_height=341, src_rowstride=6291544, src_channels=3, 
    src_has_alpha=0, scale_x=<optimized out>, scale_y=<optimized out>, check_x=0, check_y=0, check_size=0, color1=0, color2=0, filter=0x7ffffffead20, 
    line_func=0x7fffedf1dcf0 <scale_line>, pixel_func=0x7fffedf1eac0 <scale_pixel>) at pixops.c:1351
#5  0x00007fffedf1fe17 in _pixops_scale_real (interp_type=PIXOPS_INTERP_BILINEAR, scale_y=0,0029325513196480938, scale_x=8,5829501602389108e-05, 
    src_has_alpha=0, src_channels=3, src_rowstride=6291544, src_height=341, src_width=2097181, src_buf=0x7fff38f00000 "", dest_has_alpha=0, 
    dest_channels=3, dest_rowstride=540, render_y1=1, render_x1=0, render_y0=0, render_x0=0, dest_buf=0x7fffc58a3800 'Z' <repeats 200 times>...)
    at pixops.c:2299
#6  _pixops_scale (dest_buf=dest_buf@entry=0x7fffc58a3800 'Z' <repeats 200 times>..., dest_width=<optimized out>, dest_height=<optimized out>, 
    dest_rowstride=540, dest_channels=3, dest_has_alpha=0, src_buf=0x7fff38f00000 "", src_width=2097181, src_height=341, src_rowstride=6291544, 
    src_channels=3, src_has_alpha=0, dest_x=0, dest_y=0, dest_region_width=180, dest_region_height=1, offset_x=offset_x@entry=0, 
    offset_y=offset_y@entry=0, scale_x=scale_x@entry=8,5829501602389108e-05, scale_y=scale_y@entry=0,0029325513196480938, 
    interp_type=PIXOPS_INTERP_BILINEAR) at pixops.c:2354
#7  0x00007fffedf17bb9 in gdk_pixbuf_scale (src=0x7fffd889ce40, dest=0x7fffd889b9e0, dest_x=0, dest_y=0, dest_width=180, dest_height=1, offset_x=0, 
    offset_y=0, scale_x=8,5829501602389108e-05, scale_y=0,0029325513196480938, interp_type=GDK_INTERP_BILINEAR) at gdk-pixbuf-scale.c:156
#8  0x00007fffedf18176 in gdk_pixbuf_scale_simple (src=src@entry=0x7fffd889ce40, dest_width=180, dest_height=dest_height@entry=1, 
    interp_type=interp_type@entry=GDK_INTERP_BILINEAR) at gdk-pixbuf-scale.c:344
#9  0x00007fffedf194cd in get_scaled_pixbuf (scaled=0x7fffcacf0ec0, pixbuf=0x7fffd889ce40) at gdk-pixbuf-scaled-anim.c:138
#10 0x00007fffedf14d94 in gdk_pixbuf_new_from_file_at_scale (filename=0x7fffc258e640 "/home/g/ready/a/boom.bmp", width=<optimized out>, 
    height=<optimized out>, preserve_aspect_ratio=<optimized out>, error=0x0) at gdk-pixbuf-io.c:1401
#11 0x00007ffff2a43113 in ?? () from /home/g/Apps/firefox/libxul.so
#12 0x00007fffef4db3b8 in g_closure_invoke () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#13 0x00007fffef4ecd3d in ?? () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#14 0x00007fffef4f4a29 in g_signal_emit_valist () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#15 0x00007fffef4f5212 in g_signal_emit_by_name () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#16 0x00007fffef4db3b8 in g_closure_invoke () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#17 0x00007fffef4ecd3d in ?? () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#18 0x00007fffef4f4a29 in g_signal_emit_valist () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#19 0x00007fffef4f5212 in g_signal_emit_by_name () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#20 0x00007fffef4db3b8 in g_closure_invoke () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#21 0x00007fffef4ecd3d in ?? () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#22 0x00007fffef4f4a29 in g_signal_emit_valist () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#23 0x00007fffef4f5212 in g_signal_emit_by_name () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#24 0x00007fffeec5d0e7 in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
---Type <return> to continue, or q <return> to quit---
#25 0x00007fffeec603f0 in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#26 0x00007fffef4db5e7 in ?? () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#27 0x00007fffef4f4088 in g_signal_emit_valist () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#28 0x00007fffef4f4ce2 in g_signal_emit () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#29 0x00007fffeeda82be in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#30 0x00007fffeedabb86 in gtk_tree_view_set_cursor_on_cell () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#31 0x00007fffeec61fb6 in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#32 0x00007fffef4de372 in g_cclosure_marshal_VOID__POINTERv () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#33 0x00007fffef4db5e7 in ?? () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#34 0x00007fffef4f4088 in g_signal_emit_valist () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#35 0x00007fffef4f4ce2 in g_signal_emit () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#36 0x00007fffeec7361a in ?? () from /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0
#37 0x00007fffee63f6e7 in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#38 0x00007fffee6728db in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#39 0x00007fffee6728f9 in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#40 0x00007fffef20bce5 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#41 0x00007fffef20c048 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#42 0x00007fffef20c0ec in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#43 0x00007ffff175238f in ?? () from /home/g/Apps/firefox/libxul.so
#44 0x00007ffff175196d in ?? () from /home/g/Apps/firefox/libxul.so
#45 0x00007ffff169f390 in ?? () from /home/g/Apps/firefox/libxul.so
#46 0x00007ffff16a71a9 in ?? () from /home/g/Apps/firefox/libxul.so
#47 0x00007ffff16be8f3 in ?? () from /home/g/Apps/firefox/libxul.so
#48 0x00007ffff1bc9fd2 in ?? () from /home/g/Apps/firefox/libxul.so
#49 0x00007ffff2a25655 in ?? () from /home/g/Apps/firefox/libxul.so
#50 0x00007ffff2f248f3 in ?? () from /home/g/Apps/firefox/libxul.so
#51 0x00007ffff2f58645 in ?? () from /home/g/Apps/firefox/libxul.so
#52 0x00007ffff2f5accb in ?? () from /home/g/Apps/firefox/libxul.so
#53 0x00007ffff2f5af66 in XRE_main () from /home/g/Apps/firefox/libxul.so
#54 0x00000000004085eb in _start ()

Comment 16

[Gustavo Grieco]

Attachment: This test case is enough to abort Firefox 40 in my netbook (it has 2GB of RAM)

Comment 17

[Lee Salzman [:lsalzman]]

That crash is not happening in the same place for the same reason. It's just signaling an OOM now instead of an overflow.

Comment 18

[Gustavo Grieco]

(In reply to Lee Salzman [:eihrul] from comment #17)
> That crash is not happening in the same place for the same reason. It's just
> signaling an OOM now instead of an overflow.

Yes, but i thought the fix will avoid such abort too.

Comment 19

[Lee Salzman [:lsalzman]]

Attachment: explicitly calculate gdk pixbuf scaled preview size

After digging further into Gustavo's info, I found that my calculations of the scaled size were off, due to severely non-obvious control flow in gdk-pixbuf's scale calculation. The aspect ratio calculations were actually being triggered in the at_scale_size_prepared_cb in gdk-pixbuf-io.c, which could let an image be constructed that could still trigger the SIGSEGV based on my old patch.

I've adjusted the calculations to take this into account, so that we now explicitly pass in the desired scaled size including aspect ratio and no longer let gdk have control of that. This way we can with certainty predict if an integer overflow would occur.

Anyway, this patch overlays my previous patch to get rid of the corner case that was left.

However, my other finding is that despite this, any OOMs that happen will and indeed must cause an abort like Gustavo noticed in his recent post. The gdk-pixbuf code always assumes that either the malloc returns non-null, or has aborted, so there are no null checks in the code anywhere. Thus, our only option is to leave the abort behavior in place. This is still much safer than leaving the SIGSEGV from wild memory accesses.

An alternative mitigation strategy to also catch the abort would be to further have a maximum preview image size as well, which still would not guarantee aborts wouldn't occur in memory constrained scenarios, but at least would allow reasonable cases to pass through and filter out possibly malicious cases.

Comment 20

[Al Billings [:abillings - ex-MoCo]]

(In reply to Lee Salzman [:eihrul] from comment #19)
> 
> However, my other finding is that despite this, any OOMs that happen will
> and indeed must cause an abort like Gustavo noticed in his recent post. The
> gdk-pixbuf code always assumes that either the malloc returns non-null, or
> has aborted, so there are no null checks in the code anywhere. Thus, our
> only option is to leave the abort behavior in place. This is still much
> safer than leaving the SIGSEGV from wild memory accesses.

Does this change the security implications of the original problem at all?

Do you think we should wait to disclose this (write an advisory) until your corner cases are fixed or should I go ahead and disclose this next week when we release 40?

Comment 21

[Lee Salzman [:lsalzman]]

(In reply to Al Billings [:abillings] from comment #20)
> (In reply to Lee Salzman [:eihrul] from comment #19)
> > 
> > However, my other finding is that despite this, any OOMs that happen will
> > and indeed must cause an abort like Gustavo noticed in his recent post. The
> > gdk-pixbuf code always assumes that either the malloc returns non-null, or
> > has aborted, so there are no null checks in the code anywhere. Thus, our
> > only option is to leave the abort behavior in place. This is still much
> > safer than leaving the SIGSEGV from wild memory accesses.
> 
> Does this change the security implications of the original problem at all?
> 
> Do you think we should wait to disclose this (write an advisory) until your
> corner cases are fixed or should I go ahead and disclose this next week when
> we release 40?

You can still construct images that force gdk-pixbuf to allocate 2GB or more
of memory without causing the original overflow, and such allocations are almost
guaranteed to OOM on the systems most users have, thus causing Firefox to abort.
This is probably not desirable, since while it's not a threat of leaking user
information anymore, it can still deny service.

If we were to limit maximum image preview sizes to, say, no bigger than 4096
on either dimension, then the maximum possible allocation would end up being
only 4096*4096*4, or about 64MB, which is unlikely to cause an OOM, and it would
also have the side-effect of preventing the original overflow from occurring
since the scale ratios would never exceed what causes the scaling code go bug out.
We'd just unfortunately be stuck with that limit until we replaced gdk-pixbuf
in the future.

That might be the best/easiest overall mitigation and could be done easily before
next week if you're okay with that proposal... Then it would be safe enough to
disclose since we'd have a fairly decent kluge-around in place.

Comment 22

[Al Billings [:abillings - ex-MoCo]]

We've already built release candidates (for the third time!) so we're not going to take more fixes. I think we should fix this in 41 with your suggestions and not disclose this yet next week.

Dan, do you concur?

Comment 23

[Lee Salzman [:lsalzman]]

Attachment: limit gtk file picker preview source size

Okay, here's the simpler/stupider/more fool-proof alternate approach that just limits the source size to less than 4096 on any dimension. The maximum buffer it will thus allocate in this case is going to be around 4*4096^2, so 64MB, and thus not terribly likely to OOM.

As a side-effect, this will also prevent the scale ratio filtering overflow without having to explicitly check for it.

Kill two vulnerabilities with one patch.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8645021 [details] [diff] [review]
limit gtk file picker preview source size

https://hg.mozilla.org/integration/mozilla-inbound/rev/10e77092a656

So I guess this is going to need Aurora/Beta/esr38 approval requests as well? Note that this push is highly unlikely to be merged to m-c until after today's uplifts. Whatever that means for the esr38 et al tracking status.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/10e77092a656

Comment 26

[Daniel Veditz [:dveditz]]

We should have cloned this bug for the new patch once we knew it was a different crash that wasn't the same security bug, and especially when it had to land in a different release. Status here is messy! Since the follow-up won't need a new security advisory I guess we're OK.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bef1e3e26eee
https://hg.mozilla.org/releases/mozilla-beta/rev/6b5922886652

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/0daaaa5cb0ab

Comment 29

[Kent James (:rkent)]

I'm trying to decide whether to take this bug, and also bug 1031145 (which I am not authorized to view) in Thunderbird 38.2.0  Normally we would to match our code as closely as possible to the Firefox esr38.2.0 release, which has already shipped.

Is there any expectation of doing a FF 38.2.1 due to this or the other bug? Thoughts on whether this might be important for Thunderbird?

Comment 30

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Removing qe-verify+ flag since we were unable to setup the test environment for this (see comment 13).

Comment 31

[Matt Wobensmith [:mwobensmith][:matt:]]

Hello Gustavo, would you mind taking a look at this using a recent build of Fx41 to see if we've fixed this issue for your scenario? Thank you.

Comment 32

[Gustavo Grieco]

Sure. I will test it as soon as possible, later today. Do you have a link to a Firefox 41 Linux build?

Comment 33

[Matt Wobensmith [:mwobensmith][:matt:]]

Here you go:

http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/41.0b5-candidates/build1/

Comment 34

[Gustavo Grieco]

I'm affraid that build is too unstable. It keep crashing as soon as i try to browse.
Here you have the stack trace:

Program received signal SIG38, Real-time event 38.
0x00007ffff09a80d0 in ?? () from /usr/lib/x86_64-linux-gnu/libfreetype.so.6
(gdb) bt
#0  0x00007ffff09a80d0 in ?? () from /usr/lib/x86_64-linux-gnu/libfreetype.so.6
#1  0x00007ffff096484c in FT_Load_Glyph () from /usr/lib/x86_64-linux-gnu/libfreetype.so.6
#2  0x00007ffff307ad83 in vpx_get_mb_ss_mmx () from /home/g/Apps/firefox/libxul.so
#3  0x00007ffff16b926f in ?? () from /home/g/Apps/firefox/libxul.so
#4  0x00007ffff3093f4b in vpx_get_mb_ss_mmx () from /home/g/Apps/firefox/libxul.so
#5  0x00007ffff30b429e in vpx_get_mb_ss_mmx () from /home/g/Apps/firefox/libxul.so
#6  0x00007ffff30a1448 in vpx_get_mb_ss_mmx () from /home/g/Apps/firefox/libxul.so
#7  0x00007ffff30b631f in vpx_get_mb_ss_mmx () from /home/g/Apps/firefox/libxul.so
#8  0x00007ffff158e87f in ?? () from /home/g/Apps/firefox/libxul.so
#9  0x00007ffff1e72731 in ?? () from /home/g/Apps/firefox/libxul.so
#10 0x00007ffff15a7544 in ?? () from /home/g/Apps/firefox/libxul.so
...

It looks completely unrelated. I guess we should wait for a better build..

Comment 35

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Gustavo Grieco from comment #34)
> I'm affraid that build is too unstable. It keep crashing as soon as i try to
> browse.

Gustavo, I'm not sure why the build was unstable for you, but could you also try the following builds?
- 41 beta 7 - http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/41.0b7-candidates/build1/
- latest ESR - http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-esr38-linux64/latest/

If you can't get the time or still have issues, then we'll also give it a try.

Comment 36

[Gustavo Grieco]

I can tested early next week. If i'm getting the same crashes as my previous comment, i will open a new bug.

Comment 37

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Gustavo Grieco from comment #36)
> I can tested early next week. If i'm getting the same crashes as my previous
> comment, i will open a new bug.

Sounds perfect, thanks!

Comment 38

[Lee Salzman [:lsalzman]]

(In reply to Gustavo Grieco from comment #34)
> I'm affraid that build is too unstable. It keep crashing as soon as i try to
> browse.
> Here you have the stack trace:
> 
> Program received signal SIG38, Real-time event 38.
> 0x00007ffff09a80d0 in ?? () from /usr/lib/x86_64-linux-gnu/libfreetype.so.6
> (gdb) bt
> #0  0x00007ffff09a80d0 in ?? () from
> /usr/lib/x86_64-linux-gnu/libfreetype.so.6
> #1  0x00007ffff096484c in FT_Load_Glyph () from
> /usr/lib/x86_64-linux-gnu/libfreetype.so.6
> #2  0x00007ffff307ad83 in vpx_get_mb_ss_mmx () from
> /home/g/Apps/firefox/libxul.so
> #3  0x00007ffff16b926f in ?? () from /home/g/Apps/firefox/libxul.so
> #4  0x00007ffff3093f4b in vpx_get_mb_ss_mmx () from
> /home/g/Apps/firefox/libxul.so
> #5  0x00007ffff30b429e in vpx_get_mb_ss_mmx () from
> /home/g/Apps/firefox/libxul.so
> #6  0x00007ffff30a1448 in vpx_get_mb_ss_mmx () from
> /home/g/Apps/firefox/libxul.so
> #7  0x00007ffff30b631f in vpx_get_mb_ss_mmx () from
> /home/g/Apps/firefox/libxul.so
> #8  0x00007ffff158e87f in ?? () from /home/g/Apps/firefox/libxul.so
> #9  0x00007ffff1e72731 in ?? () from /home/g/Apps/firefox/libxul.so
> #10 0x00007ffff15a7544 in ?? () from /home/g/Apps/firefox/libxul.so
> ...
> 
> It looks completely unrelated. I guess we should wait for a better build..

SIG38 does not mean a crash; it's a signal uses inside Firefox for internal communication. Just have GDB ignore it or continue past it.

Comment 39

[Gustavo Grieco]

It works great now. I tested with all the broken images, and i got no crash. Even with new heap overflow i reported today (https://bugzilla.mozilla.org/show_bug.cgi?id=1203078). The only crash i could produce was using the DoS with the TARGA file, because it is affecting the parsing of the image itself in gdk (so, there is no way to prevent it unless you disable the TARGA loader).

Comment 40

[Matt Wobensmith [:mwobensmith][:matt:]]

Marking verified based on comment 39.

Comment 41

[Gustavo Grieco]

I think this fix should be pushed as fast as possible. More vulnerabilities are waiting inside gdk-pixbuf, even some are public like this one: https://bugzilla.gnome.org/show_bug.cgi?id=734556