Closed Bug 1184009 (CVE-2015-4491) Opened 9 years ago Closed 9 years ago

gdk-pixbuf heap overflow and DoS affecting Firefox

Categories

(Core :: Widget: Gtk, defect)

39 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla43
Tracking Status
firefox39 --- wontfix
firefox40 - wontfix
firefox41 + verified
firefox42 + fixed
firefox43 --- fixed
firefox-esr31 --- wontfix
firefox-esr38 41+ fixed
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- unaffected
b2g-v2.2r --- unaffected
b2g-master --- unaffected

People

(Reporter: gustavo.grieco, Assigned: lsalzman)

References

Details

(Keywords: csectype-bounds, sec-high, Whiteboard: [adv-main40+][adv-esr38.2+][adv-main41-][adv-esr38.3-])

Attachments

(4 files, 1 obsolete file)

Attached file overflow-firefox.bmp.gz (deleted) —
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)
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Assignee: nobody → lsalzman
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: sec-high
Attached patch limit image preview sizes (deleted) — — Splinter Review
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.
Attachment #8636664 - Flags: sec-approval?
Attachment #8636664 - Flags: review?(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.
Attachment #8636664 - Flags: review?(acomminos) → review+
(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.
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.
Attachment #8636664 - Flags: sec-approval? → sec-approval+
(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?
Flags: needinfo?(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.
Flags: needinfo?(lsalzman)
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.
Attachment #8636664 - Flags: approval-mozilla-esr38?
Attachment #8636664 - Flags: approval-mozilla-beta?
Attachment #8636664 - Flags: approval-mozilla-aurora?
Flags: sec-bounty?
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+
Attachment #8636664 - Flags: approval-mozilla-esr38?
Attachment #8636664 - Flags: approval-mozilla-esr38+
Attachment #8636664 - Flags: approval-mozilla-beta?
Attachment #8636664 - Flags: approval-mozilla-beta+
Attachment #8636664 - Flags: approval-mozilla-aurora?
Attachment #8636664 - Flags: approval-mozilla-aurora+
Alias: CVE-2015-4491
Whiteboard: [adv-main40+][adv-esr38.2+]
Flags: qe-verify+
Flags: sec-bounty? → sec-bounty+
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/
Flags: needinfo?(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/
Flags: needinfo?(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 ()
That crash is not happening in the same place for the same reason. It's just signaling an OOM now instead of an overflow.
(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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch explicitly calculate gdk pixbuf scaled preview size (obsolete) (deleted) — — Splinter Review
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.
Flags: needinfo?(abillings)
Attachment #8644743 - Flags: review?(acomminos)
(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?
Flags: needinfo?(abillings) → needinfo?(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.
Flags: needinfo?(lsalzman)
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?
Flags: needinfo?(dveditz)
Attachment #8644743 - Flags: review?(acomminos) → review+
Attached patch limit gtk file picker preview source size (deleted) — — Splinter Review
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.
Attachment #8644743 - Attachment is obsolete: true
Attachment #8645021 - Flags: review?(acomminos)
Attachment #8645021 - Flags: review?(acomminos) → review+
Attachment #8645021 - Flags: checkin?
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.
Attachment #8645021 - Flags: checkin? → checkin+
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: mozilla42 → mozilla43
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.
Flags: needinfo?(dveditz)
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?
Removing qe-verify+ flag since we were unable to setup the test environment for this (see comment 13).
Flags: qe-verify+
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.
Flags: needinfo?(gustavo.grieco)
Sure. I will test it as soon as possible, later today. Do you have a link to a Firefox 41 Linux build?
Flags: needinfo?(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..
Group: core-security → core-security-release
(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.
Flags: needinfo?(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.
Flags: needinfo?(gustavo.grieco)
(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!
(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.
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).
Marking verified based on comment 39.
Status: RESOLVED → VERIFIED
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
Whiteboard: [adv-main40+][adv-esr38.2+] → [adv-main40+][adv-esr38.2+][adv-main41-]
Whiteboard: [adv-main40+][adv-esr38.2+][adv-main41-] → [adv-main40+][adv-esr38.2+][adv-main41-][adv-esr38.3-]
Group: core-security-release
Attachment #8643255 - Attachment description: gustavo.grieco@imag.fr,4000?,2015-07-15,2015-07-30,2015-08-04,true,,, → gustavo.grieco@imag.fr,4000,2015-07-15,2015-07-30,2015-08-04,true,,,
Attachment #8643255 - Attachment description: gustavo.grieco@imag.fr,4000,2015-07-15,2015-07-30,2015-08-04,true,,, → gustavo.grieco@imag.fr,4000,2015-07-15,2015-07-30,2015-08-04,true,Gustavo Grieco,,
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: