Created attachment 114208 [details] (PDF file used in the test code that triggers the crash) (this is using cairo 1.14.0 and poppler 0.32.0) I've created a minimal test case that reliably reproduces the crash. It crashes on both Windows and OS X. However, on Windows it crashes at window size (1202, 931) while on OS X it crashes at window size (1207,932). It also seems to be data related because the test PDF has two pages and it only crashes when rendering page 2. The test PDF is attached. Here is the test code: #include <poppler.h> #include <stdio.h> // printf() int main(int argc, char **argv) { PopplerDocument *doc; PopplerPage *page; cairo_surface_t *surface; cairo_t *cr; double x_points, y_points; // measured in points which are 1/72 inch double pdf_width, pdf_height; // measured in pixels double window_width, window_height; // measured in pixels int fit_width; double scale_factor; double x_padding; double y_padding; #ifdef __APPLE__ doc = poppler_document_new_from_file("file:///Users/draymond/crash/test.pdf", NULL, NULL); #else doc = poppler_document_new_from_file("file:///C:/crash/test.pdf", NULL, NULL); #endif page = poppler_document_get_page(doc, 1); // does not crash rendering page 0 for (window_width = 1200; window_width <= 1210; window_width++) { for (window_height = 930; window_height <= 940; window_height++) { printf("(%f, %f)\n", window_width, window_height); poppler_page_get_size(page, &x_points, &y_points); // 792, 612 fit_width = (window_width / window_height) < (x_points / y_points); scale_factor = fit_width ? (window_width / x_points) : (window_height / y_points); pdf_width = x_points * scale_factor; pdf_height = y_points * scale_factor; x_padding = fit_width ? 0 : ((window_width - pdf_width) / 2); y_padding = fit_width ? ((window_height - pdf_height) / 2) : 0; surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, window_width, window_height); cr = cairo_create(surface); cairo_translate(cr, x_padding, y_padding); cairo_scale(cr, scale_factor, scale_factor); poppler_page_render(page, cr); cairo_destroy(cr); cairo_surface_destroy(surface); } } printf("success\n"); g_object_unref(page); g_object_unref(doc); }
Here is more GDB info showing the faulting instruction, the contents of the bad pointer, and the full backtrace: Program received signal SIGSEGV, Segmentation fault. 0x68fe680d in _fill_xrgb32_lerp_opaque_spans () from C:\msys2\usr\local\bin\libcairo-2.dll (gdb) x/i $pc => 0x68fe680d <_fill_xrgb32_lerp_opaque_spans+281>: mov (%eax),%ecx (gdb) info registers eax 0x30b5000 51073024 ecx 0x0 0 edx 0xfff57b52 -689326 ebx 0x165 357 esp 0x28d960 0x28d960 ebp 0x28d9c8 0x28d9c8 esi 0x194 404 edi 0x28da80 2677376 eip 0x68fe680d 0x68fe680d <_fill_xrgb32_lerp_opaque_spans+281> eflags 0x10286 [ PF SF IF RF ] cs 0x23 35 ss 0x2b 43 ds 0x2b 43 es 0x2b 43 fs 0x53 83 gs 0x2b 43 (gdb) bt #0 0x68fe680d in _fill_xrgb32_lerp_opaque_spans () from C:\msys2\usr\local\bin\libcairo-2.dll #1 0x69038015 in blit_a8 () from C:\msys2\usr\local\bin\libcairo-2.dll #2 0x69038593 in glitter_scan_converter_render () from C:\msys2\usr\local\bin\libcairo-2.dll #3 0x6903872e in _cairo_tor_scan_converter_generate () from C:\msys2\usr\local\bin\libcairo-2.dll #4 0x690279ae in composite_polygon () from C:\msys2\usr\local\bin\libcairo-2.dll #5 0x69027e5b in clip_and_composite_polygon () from C:\msys2\usr\local\bin\libcairo-2.dll #6 0x690281fc in _cairo_spans_compositor_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #7 0x68fd7386 in _cairo_compositor_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #8 0x68fecf54 in _cairo_image_surface_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #9 0x6902cb09 in _cairo_surface_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #10 0x68fdfdad in _cairo_gstate_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #11 0x68fda87c in _cairo_default_context_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #12 0x68fd0d04 in cairo_stroke () from C:\msys2\usr\local\bin\libcairo-2.dll #13 0x6cc1b143 in CairoOutputDev::stroke (this=0x289e5d8, state=0x30e6ea8) at CairoOutputDev.cc:783 #14 0x008764ff in Gfx::opStroke (this=0x31079c0, args=0x28fa80, numArgs=0) at Gfx.cc:1856 #15 0x0086a599 in Gfx::execOp (this=this at entry=0x31079c0, cmd=cmd at entry=0x28fa58, args=args at entry=0x28fa80, numArgs=numArgs at entry=0) at Gfx.cc:904 #16 0x00872c3f in Gfx::go (this=this at entry=0x31079c0, topLevel=topLevel at entry=true) at Gfx.cc:763 #17 0x0087312e in Gfx::display (this=0x31079c0, obj=0x28fd80, topLevel=true) at Gfx.cc:729 #18 0x008b5849 in Page::displaySlice (this=0x289fda8, out=0x289e5d8, hDPI=72, vDPI=72, rotate=0, useMediaBox=false, crop=true, sliceX=-1, sliceY=-1, sliceW=-1, sliceH=-1, printing=false, abortCheckCbk=0x0, abortCheckCbkData=0x0, annotDisplayDecideCbk=0x0, annotDisplayDecideCbkData=0x0, copyXRef=false) at Page.cc:585 #19 0x6cc07e67 in _poppler_page_render (page=0x2867020, cairo=0x28d4fe8, printing=<optimized out>, print_flags=POPPLER_PRINT_DOCUMENT) at poppler-page.cc:362 #20 0x00401840 in main ()
I have confirmed that the change submitted below suppresses the crash during my test case: http://cgit.freedesktop.org/cairo/commit/?id=5c82d91a5e15d29b1489dcb413b24ee7fdf59934 This change was pulled into cairo 1.14.2. However, the original author of that patch (ilia.softway@gmail.com) has expressed doubts that this change fully fixes the problem and he is now using an additional patch to cell_list_add_subspan() in cairo-tor_scan-converter.c. Therefore, I think it is best to leave this bug open until the root cause is fully understood.
Created attachment 125303 [details] [review] similar to [5c82d91] in other fill/blit functions where len is signed and may go negative
Created attachment 125304 [details] [review] similar to [5c82d91] in other fill/blit functions where len is signed and may go negative
Any news on the reporter's further work? While reviewing this change for a backport I noticed a couple of other obvious cases of potentially the same flaw, where (len--) is used to bound a loop and len is signed int which may go negative. Attached two patches dealing with these cases.
The backtrace you posted unfortunately is against a cairo built without symbols, so it's not clear where the crash within Cairo happened. What Ilia might be referring to is that the fix is suitably defensive but doesn't address why the value is negative in the first place. So, the patch probably did actually fix the crashing behavior, but there may also need to be some input value checking added somewhere. I've opted to squash the two patches and land them, as they're also adding defensive programming checks. I'll leave the bug open for now, if others wish to investigate why the invalid inputs are entering Cairo. commit 63f14d4a8f155ebaaca63b49e7bacca55d681af5 Author: Doran Moppert <dmoppert@redhat.com> AuthorDate: Mon Jul 25 11:00:21 2016 +0930 Commit: Bryce Harrington <bryce@osg.samsung.com> CommitDate: Mon Aug 21 17:08:47 2017 -0700 image: Check for negative len in fill/blit functions Applies the same fix as 5c82d91 to other potential negative len cases. Reviewed-by: Bryce Harrington <bryce@osg.samsung.com>
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/157.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.