commit ebf0b569a63f15b5dc7532f16936104af1e09f02 Author: Andrzej Hunt Date: Tue Jul 16 18:30:28 2024 +0200 xcf: don't use potentially dangling pointer in xcf_load_layer_mask layer_mask points to the original mask created by xcf_load_layer_mask. We copy this pointer into channel, and xcf_load_channel_props can overwrite this pointer and free the original mask. If this happens, layer_mask points to the now-freed original mask, and should not be used. Therefore we need to change later parts of xcf_load_layer_mask to use channel instead of layer_mask. Additionally, we add a block and move layer_mask into this block to guarantee that layer_mask cannot be used after it has potentially been freed. Adjustments by Jacob Boerema: Follow GIMP's code style regarding variables, comment style and position of braces See also ASAN output below: ==5247==ERROR: AddressSanitizer: heap-use-after-free on address 0x615000010fd0 at pc 0x7f4e2dbbf31b bp 0x7ffca8a95cd0 sp 0x7ffca8a95cc8 READ of size 8 at 0x615000010fd0 thread T0 #0 0x7f4e2dbbf31a in g_type_check_instance_cast /home/ahunt/git/glib/_build/../gobject/gtype.c:4117:26 #1 0xb200fe in xcf_load_layer_mask /home/ahunt/git/gimp/app/xcf/xcf-load.c:2305:52 #2 0xb18eea in xcf_load_layer /home/ahunt/git/gimp/app/xcf/xcf-load.c:2133:20 #3 0xb13d91 in xcf_load_image /home/ahunt/git/gimp/app/xcf/xcf-load.c:499:15 #4 0xb11deb in xcf_load_stream /home/ahunt/git/gimp/app/xcf/xcf.c:305:19 #5 0x619dfd in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/app/fuzzers/xcf_fuzzer.c:50:17 #6 0x51d364 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15 #7 0x506fe2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6 #8 0x50d350 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:9 #9 0x5373a2 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 #10 0x7f4e2c84c349 in __libc_start_main (/lib64/libc.so.6+0x24349) #11 0x4e0779 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 0x615000010fd0 is located 336 bytes inside of 504-byte region [0x615000010e80,0x615000011078) freed by thread T0 here: #0 0x5e8562 in free /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3 #1 0x7f4e2d76ce08 in g_free /home/ahunt/git/glib/_build/../glib/gmem.c:199:3 #2 0x7f4e2d797a6b in g_slice_free1 /home/ahunt/git/glib/_build/../glib/gslice.c:1183:7 #3 0x7f4e2dbb7b04 in g_type_free_instance /home/ahunt/git/glib/_build/../gobject/gtype.c:2008:5 #4 0x7f4e2db8fe3a in g_object_unref /home/ahunt/git/glib/_build/../gobject/gobject.c:3604:11 #5 0xb22fff in xcf_load_channel_props /home/ahunt/git/gimp/app/xcf/xcf-load.c:1738:13 #6 0xb20037 in xcf_load_layer_mask /home/ahunt/git/gimp/app/xcf/xcf-load.c:2292:9 #7 0xb18eea in xcf_load_layer /home/ahunt/git/gimp/app/xcf/xcf-load.c:2133:20 #8 0xb13d91 in xcf_load_image /home/ahunt/git/gimp/app/xcf/xcf-load.c:499:15 #9 0xb11deb in xcf_load_stream /home/ahunt/git/gimp/app/xcf/xcf.c:305:19 #10 0x619dfd in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/app/fuzzers/xcf_fuzzer.c:50:17 #11 0x51d364 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15 #12 0x506fe2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6 #13 0x50d350 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:9 #14 0x5373a2 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 #15 0x7f4e2c84c349 in __libc_start_main (/lib64/libc.so.6+0x24349) previously allocated by thread T0 here: #0 0x5e87cd in malloc /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x7f4e2d76ccf2 in g_malloc /home/ahunt/git/glib/_build/../glib/gmem.c:106:13 #2 0x7f4e2d7972e0 in g_slice_alloc /home/ahunt/git/glib/_build/../glib/gslice.c:1072:11 #3 0x7f4e2d7978ae in g_slice_alloc0 /home/ahunt/git/glib/_build/../glib/gslice.c:1098:18 #4 0x7f4e2dbb6e0a in g_type_create_instance /home/ahunt/git/glib/_build/../gobject/gtype.c:1911:17 #5 0x7f4e2db9215e in g_object_new_internal /home/ahunt/git/glib/_build/../gobject/gobject.c:1945:24 #6 0x7f4e2db91d1f in g_object_new_valist /home/ahunt/git/glib/_build/../gobject/gobject.c:2288:16 #7 0x7f4e2db90e8b in g_object_new /home/ahunt/git/glib/_build/../gobject/gobject.c:1788:12 #8 0xdb69e0 in gimp_item_new /home/ahunt/git/gimp/app/core/gimpitem.c:723:10 #9 0xce11c8 in gimp_drawable_new /home/ahunt/git/gimp/app/core/gimpdrawable.c:1067:14 #10 0xddf5d8 in gimp_layer_mask_new /home/ahunt/git/gimp/app/core/gimplayermask.c:254:5 #11 0xb1ffc5 in xcf_load_layer_mask /home/ahunt/git/gimp/app/xcf/xcf-load.c:2279:31 #12 0xb18eea in xcf_load_layer /home/ahunt/git/gimp/app/xcf/xcf-load.c:2133:20 #13 0xb13d91 in xcf_load_image /home/ahunt/git/gimp/app/xcf/xcf-load.c:499:15 #14 0xb11deb in xcf_load_stream /home/ahunt/git/gimp/app/xcf/xcf.c:305:19 #15 0x619dfd in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/app/fuzzers/xcf_fuzzer.c:50:17 #16 0x51d364 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15 #17 0x506fe2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:6 #18 0x50d350 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:9 #19 0x5373a2 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 #20 0x7f4e2c84c349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: heap-use-after-free /home/ahunt/git/glib/_build/../gobject/gtype.c:4117:26 in g_type_check_instance_cast Shadow bytes around the buggy address: 0x0c2a7fffa1a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffa1b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffa1c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2a7fffa1d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2a7fffa1e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x0c2a7fffa1f0: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd 0x0c2a7fffa200: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa 0x0c2a7fffa210: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2a7fffa220: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2a7fffa230: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2a7fffa240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==5247==ABORTING ./crash-0507799c3e4291570e060f53332b58b8a96f95e5 (cherry picked from commit fe26086e16943860f3852120f546ce913a7a73ee) # Conflicts: # app/xcf/xcf-load.c diff --git a/app/xcf/xcf-load.c b/app/xcf/xcf-load.c index da196bcdb2..41e582719a 100644 --- a/app/xcf/xcf-load.c +++ b/app/xcf/xcf-load.c @@ -102,7 +102,8 @@ static gboolean xcf_check_layer_props (XcfInfo *info, gboolean *is_text_layer); static gboolean xcf_load_channel_props (XcfInfo *info, GimpImage *image, - GimpChannel **channel); + GimpChannel **channel, + gboolean is_mask); static gboolean xcf_load_prop (XcfInfo *info, PropType *prop_type, guint32 *prop_size); @@ -1656,7 +1657,8 @@ xcf_check_layer_props (XcfInfo *info, static gboolean xcf_load_channel_props (XcfInfo *info, GimpImage *image, - GimpChannel **channel) + GimpChannel **channel, + gboolean is_mask) { PropType prop_type; guint32 prop_size; @@ -1679,6 +1681,14 @@ xcf_load_channel_props (XcfInfo *info, { GimpChannel *mask; + if (is_mask) + { + /* PROP_SELECTION is not valid for masks, and we have to avoid + * overwriting the channel. + */ + continue; + } + /* We're going to delete *channel, Don't leave its pointer * in @info. See bug #767873. */ @@ -2151,7 +2161,7 @@ xcf_load_channel (XcfInfo *info, return NULL; /* read in the channel properties */ - if (! xcf_load_channel_props (info, image, &channel)) + if (! xcf_load_channel_props (info, image, &channel, FALSE)) goto error; xcf_progress_update (info); @@ -2238,7 +2248,7 @@ xcf_load_layer_mask (XcfInfo *info, /* read in the layer_mask properties */ channel = GIMP_CHANNEL (layer_mask); - if (! xcf_load_channel_props (info, image, &channel)) + if (! xcf_load_channel_props (info, image, &channel, TRUE)) goto error; xcf_progress_update (info);