After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 696331 - png: Image read memory error
png: Image read memory error
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: loaders
unspecified
Other All
: Normal critical
: ---
Assigned To: gdk-pixbuf-maint
gdk-pixbuf-maint
Depends on:
Blocks:
 
 
Reported: 2013-03-21 21:06 UTC by SztfG
Modified: 2016-12-28 15:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Valgrind output for eog (2.08 KB, text/plain)
2013-03-21 21:06 UTC, SztfG
  Details
corrupted image (12.00 KB, application/octet-stream)
2013-03-22 11:38 UTC, SztfG
  Details
tests: Add test for bug 696331 (16.27 KB, patch)
2016-12-19 18:47 UTC, Bastien Nocera
none Details | Review
tests: Add small tool to save a reference image (3.51 KB, patch)
2016-12-28 15:46 UTC, Bastien Nocera
committed Details | Review
tests: Add test for bug 696331 (22.14 KB, patch)
2016-12-28 15:46 UTC, Bastien Nocera
committed Details | Review
gdk-pixbuf: Add constant for "default fill" (1013 bytes, patch)
2016-12-28 15:46 UTC, Bastien Nocera
committed Details | Review
png: Initialise the created pixbufs (1.23 KB, patch)
2016-12-28 15:46 UTC, Bastien Nocera
committed Details | Review
tests: Fill the default pixbuf with the placeholder colour (1.18 KB, patch)
2016-12-28 15:46 UTC, Bastien Nocera
committed Details | Review

Description SztfG 2013-03-21 21:06:47 UTC
Created attachment 239498 [details]
Valgrind output for eog

If I open some image in program that use gdk-pixbuf (eye of gnome or f-spot for example), and move to this https://developer.pidgin.im/raw-attachment/ticket/15558/error.png corrupted image png image using next/previous, I got some garbage in this image. See my bugreport for pidgin https://developer.pidgin.im/ticket/15558
Comment 1 Matthias Clasen 2013-03-22 11:29:40 UTC
to get anywhere here, you will have to attach the problematic image file
Comment 2 SztfG 2013-03-22 11:32:30 UTC
(In reply to comment #1)
> to get anywhere here, you will have to attach the problematic image file
Here it is https://developer.pidgin.im/raw-attachment/ticket/15558/error.png
Comment 3 SztfG 2013-03-22 11:38:45 UTC
Created attachment 239533 [details]
corrupted image
Comment 4 SztfG 2013-03-23 08:48:30 UTC
For now(In reply to comment #1)
> to get anywhere here, you will have to attach the problematic image file

Now you can reproduce bug? What additional info do you need? This bug can be used to execute arbitrary code?
Comment 5 Bastien Nocera 2014-10-22 17:22:14 UTC
Confirmed on gdk-pixbuf 2.31.

==23138== Use of uninitialised value of size 8
==23138==    at 0x78F5670: ??? (in /usr/lib64/liblcms2.so.2.0.6)
==23138==    by 0x78DF94F: ??? (in /usr/lib64/liblcms2.so.2.0.6)
==23138==    by 0x4E5E366: eog_image_apply_display_profile (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E88198: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x8E88F53: _g_closure_invoke_va (gclosure.c:831)
==23138==    by 0x8EA2E1D: g_signal_emit_valist (gsignal.c:3195)
==23138==    by 0x8EA3681: g_signal_emit (gsignal.c:3342)
==23138==    by 0x4E62E3A: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x9113B42: g_main_dispatch (gmain.c:3125)
==23138==    by 0x9113B42: g_main_context_dispatch (gmain.c:3734)
==23138==    by 0x9113F47: g_main_context_iterate.isra.29 (gmain.c:3805)
==23138==    by 0x9113FEB: g_main_context_iteration (gmain.c:3866)
==23138==    by 0x843F5BB: g_application_run (gapplication.c:2290)
==23138==    by 0x4011E1: main (in /usr/bin/eog)
==23138==  Uninitialised value was created by a heap allocation
==23138==    at 0x4C29BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23138==    by 0x911973E: g_try_malloc (gmem.c:242)
==23138==    by 0x871E05D: gdk_pixbuf_new (gdk-pixbuf.c:456)
==23138==    by 0x1C80B078: png_info_callback (io-png.c:655)
==23138==    by 0xD22C7B2: ??? (in /usr/lib64/libpng16.so.16.10.0)
==23138==    by 0xD22D57A: png_process_data (in /usr/lib64/libpng16.so.16.10.0)
==23138==    by 0x1C80A838: gdk_pixbuf__png_image_load_increment (io-png.c:546)
==23138==    by 0x8724F53: gdk_pixbuf_loader_write (gdk-pixbuf-loader.c:523)
==23138==    by 0x4E60284: eog_image_load (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E64536: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E628FC: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x9139DB4: g_thread_proxy (gthread.c:764)
==23138==    by 0x9620529: start_thread (pthread_create.c:310)
==23138==    by 0x993577C: clone (clone.S:109)
==23138== 
==23138== Use of uninitialised value of size 8
==23138==    at 0x78F567D: ??? (in /usr/lib64/liblcms2.so.2.0.6)
==23138==    by 0x78DF94F: ??? (in /usr/lib64/liblcms2.so.2.0.6)
==23138==    by 0x4E5E366: eog_image_apply_display_profile (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E88198: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x8E88F53: _g_closure_invoke_va (gclosure.c:831)
==23138==    by 0x8EA2E1D: g_signal_emit_valist (gsignal.c:3195)
==23138==    by 0x8EA3681: g_signal_emit (gsignal.c:3342)
==23138==    by 0x4E62E3A: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x9113B42: g_main_dispatch (gmain.c:3125)
==23138==    by 0x9113B42: g_main_context_dispatch (gmain.c:3734)
==23138==    by 0x9113F47: g_main_context_iterate.isra.29 (gmain.c:3805)
==23138==    by 0x9113FEB: g_main_context_iteration (gmain.c:3866)
==23138==    by 0x843F5BB: g_application_run (gapplication.c:2290)
==23138==    by 0x4011E1: main (in /usr/bin/eog)
==23138==  Uninitialised value was created by a heap allocation
==23138==    at 0x4C29BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23138==    by 0x911973E: g_try_malloc (gmem.c:242)
==23138==    by 0x871E05D: gdk_pixbuf_new (gdk-pixbuf.c:456)
==23138==    by 0x1C80B078: png_info_callback (io-png.c:655)
==23138==    by 0xD22C7B2: ??? (in /usr/lib64/libpng16.so.16.10.0)
==23138==    by 0xD22D57A: png_process_data (in /usr/lib64/libpng16.so.16.10.0)
==23138==    by 0x1C80A838: gdk_pixbuf__png_image_load_increment (io-png.c:546)
==23138==    by 0x8724F53: gdk_pixbuf_loader_write (gdk-pixbuf-loader.c:523)
==23138==    by 0x4E60284: eog_image_load (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E64536: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E628FC: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x9139DB4: g_thread_proxy (gthread.c:764)
==23138==    by 0x9620529: start_thread (pthread_create.c:310)
==23138==    by 0x993577C: clone (clone.S:109)
==23138== 
==23138== Use of uninitialised value of size 8
==23138==    at 0x78F5697: ??? (in /usr/lib64/liblcms2.so.2.0.6)
==23138==    by 0x78DF94F: ??? (in /usr/lib64/liblcms2.so.2.0.6)
==23138==    by 0x4E5E366: eog_image_apply_display_profile (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E88198: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x8E88F53: _g_closure_invoke_va (gclosure.c:831)
==23138==    by 0x8EA2E1D: g_signal_emit_valist (gsignal.c:3195)
==23138==    by 0x8EA3681: g_signal_emit (gsignal.c:3342)
==23138==    by 0x4E62E3A: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x9113B42: g_main_dispatch (gmain.c:3125)
==23138==    by 0x9113B42: g_main_context_dispatch (gmain.c:3734)
==23138==    by 0x9113F47: g_main_context_iterate.isra.29 (gmain.c:3805)
==23138==    by 0x9113FEB: g_main_context_iteration (gmain.c:3866)
==23138==    by 0x843F5BB: g_application_run (gapplication.c:2290)
==23138==    by 0x4011E1: main (in /usr/bin/eog)
==23138==  Uninitialised value was created by a heap allocation
==23138==    at 0x4C29BCF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23138==    by 0x911973E: g_try_malloc (gmem.c:242)
==23138==    by 0x871E05D: gdk_pixbuf_new (gdk-pixbuf.c:456)
==23138==    by 0x1C80B078: png_info_callback (io-png.c:655)
==23138==    by 0xD22C7B2: ??? (in /usr/lib64/libpng16.so.16.10.0)
==23138==    by 0xD22D57A: png_process_data (in /usr/lib64/libpng16.so.16.10.0)
==23138==    by 0x1C80A838: gdk_pixbuf__png_image_load_increment (io-png.c:546)
==23138==    by 0x8724F53: gdk_pixbuf_loader_write (gdk-pixbuf-loader.c:523)
==23138==    by 0x4E60284: eog_image_load (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E64536: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x4E628FC: ??? (in /usr/lib64/eog/libeog.so)
==23138==    by 0x9139DB4: g_thread_proxy (gthread.c:764)
==23138==    by 0x9620529: start_thread (pthread_create.c:310)
==23138==    by 0x993577C: clone (clone.S:109)
Comment 6 Bastien Nocera 2016-12-19 18:44:56 UTC
If anything, the data should probably be zero'ed if it's not going to overwrite all the memory that was given.

==9778== Use of uninitialised value of size 8
==9778==    at 0x7BA8331: ??? (in /usr/lib64/liblcms2.so.2.0.8)
==9778==    by 0x7B921D8: ??? (in /usr/lib64/liblcms2.so.2.0.8)
==9778==    by 0x7B9316C: cmsDoTransform (in /usr/lib64/liblcms2.so.2.0.8)
==9778==    by 0x4E615F3: eog_image_apply_display_profile (eog-image.c:706)
==9778==    by 0x4E94971: eog_job_load_cb (eog-window.c:1378)
==9778==    by 0x95FBAC1: g_cclosure_marshal_VOID(intXX_t &&) volatile (gmarshal.c:875)
==9778==    by 0x95F8AAA: g_closure_invoke (gclosure.c:804)
==9778==    by 0x9615FD3: signal_emit_unlocked_R (gsignal.c:3635)
==9778==    by 0x961530A: g_signal_emit_valist (gsignal.c:3391)
==9778==    by 0x961584C: g_signal_emit (gsignal.c:3447)
==9778==    by 0x4E683D5: notify_finished (eog-jobs.c:158)
==9778==    by 0x989504B: g_idle_dispatch (gmain.c:5558)
==9778==    by 0x9892524: g_main_dispatch (gmain.c:3206)
==9778==    by 0x989347A: g_main_context_dispatch (gmain.c:3869)
==9778==    by 0x989365F: g_main_context_iterate (gmain.c:3942)
==9778==    by 0x9893723: g_main_context_iteration (gmain.c:4003)
==9778==    by 0x8826143: g_application_run (gapplication.c:2381)
==9778==    by 0x4013CB: main (main.c:133)
==9778==  Uninitialised value was created by a heap allocation
==9778==    at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
==9778==    by 0x989AE25: g_try_malloc (gmem.c:242)
==9778==    by 0x989B07B: g_try_malloc_n (gmem.c:402)
==9778==    by 0x8339EF5: gdk_pixbuf_new (gdk-pixbuf.c:463)
==9778==    by 0x1DAA4198: png_info_callback (io-png.c:677)
==9778==    by 0x8B4D2F0: ??? (in /usr/lib64/libpng16.so.16.26.0)
==9778==    by 0x8B4DFCA: png_process_data (in /usr/lib64/libpng16.so.16.26.0)
==9778==    by 0x1DAA3E42: gdk_pixbuf__png_image_load_increment (io-png.c:564)
==9778==    by 0x8343AC0: gdk_pixbuf_loader_write (gdk-pixbuf-loader.c:521)
==9778==    by 0x4E6222D: eog_image_real_load (eog-image.c:1045)
==9778==    by 0x4E62A85: eog_image_load (eog-image.c:1293)
==9778==    by 0x4E693E3: eog_job_load_run (eog-jobs.c:573)
==9778==    by 0x4E68796: eog_job_run (eog-jobs.c:271)
==9778==    by 0x4E679A2: eog_job_process (eog-job-scheduler.c:153)
==9778==    by 0x4E678AA: eog_job_scheduler (eog-job-scheduler.c:128)
==9778==    by 0x98C2790: g_thread_proxy (gthread.c:784)
==9778==    by 0x9D886C9: start_thread (pthread_create.c:333)
==9778==    by 0xA0A6F6E: clone (clone.S:105)
Comment 7 Bastien Nocera 2016-12-19 18:47:04 UTC
Created attachment 342232 [details] [review]
tests: Add test for bug 696331

This does not currently fail, we'd need to run the test under valgrind.
Comment 8 SztfG 2016-12-20 04:16:47 UTC
This happens if truncate end of png file, like this:
dd if=someimage.png of=badimage.png bs=200 count=1
Comment 9 Bastien Nocera 2016-12-28 15:46:04 UTC
Created attachment 342540 [details] [review]
tests: Add small tool to save a reference image

This makes it easier to create a reference image after a bug has been
fixed. Reverting the fix is enough to show that the reftest fails
without it.
Comment 10 Bastien Nocera 2016-12-28 15:46:11 UTC
Created attachment 342541 [details] [review]
tests: Add test for bug 696331

This compares a broken file, which might have random data inside the
part of the image that was missing from the file, with our version
which will have a flat colour instead.
Comment 11 Bastien Nocera 2016-12-28 15:46:17 UTC
Created attachment 342542 [details] [review]
gdk-pixbuf: Add constant for "default fill"

This colour should be used to complete truncated images, rather than
using random garbage from uninitialised memory.
Comment 12 Bastien Nocera 2016-12-28 15:46:23 UTC
Created attachment 342543 [details] [review]
png: Initialise the created pixbufs

As we cannot easily detect truncated images when feeding data piecemeal,
initialise the empty image to avoid reading uninitialised memory where
we could not fill the pixbuf.
Comment 13 Bastien Nocera 2016-12-28 15:46:29 UTC
Created attachment 342544 [details] [review]
tests: Fill the default pixbuf with the placeholder colour

Ideally, we should only be comparing data that the loader told us it
updated, and this is kind of what we're doing, as long as we don't have
a legitimate image that uses the exact same filler colour. Unlikely
though.
Comment 14 Bastien Nocera 2016-12-28 15:46:56 UTC
Attachment 342540 [details] pushed as e96cbf2 - tests: Add small tool to save a reference image
Attachment 342541 [details] pushed as 3fc5bf6 - tests: Add test for bug 696331
Attachment 342542 [details] pushed as 4228173 - gdk-pixbuf: Add constant for "default fill"
Attachment 342543 [details] pushed as 1fb2e91 - png: Initialise the created pixbufs
Attachment 342544 [details] pushed as 39a3187 - tests: Fill the default pixbuf with the placeholder colour