File gimp-CVE-2025-48798-2.patch of Package gimp
commit e7523ed41271e48a909011b8598d496c1be642e2
Author: Jehan <jehan@girinstud.io>
Date: Mon Nov 4 00:04:40 2024 +0100
Issue #11822: fix double-free in edge cases of broken XCF.
A patch was originally contributed by Andrzej Hunt in #11822 (cf.
0002-xcf-fix-channel-s-reference-counts.patch in the report).
The diagnostic of the double PROP_SELECTION issue is right, but not the
fix which was over-reffing, hence leaking channels and buffers, in the
normal cases, just to avoid double-free in broken edge cases.
The other issue is not possible though (unreffing the image's selection
when encountering an error in xcf_load_channel()) because we explicitly
check it it's the image mask AFAICS.
I added a second test which was not double-freeing yet which deserves a
bit of stderr messaging: when 2 different channels have PROP_SELECTION
set.
Relevant text from the commit message originally contributed by Andrzej
Hunt is the following (diagnostic and ASAN output still of interest):
----------------
xcf_load_channel creates a new channel using gimp_channel_new. This
channel has a floating reference (because GimpChannel is a subclass of
GimpItem, and gimp_item_init uses g_object_force_floating()).
Next, three different scenarios can occur:
- xcf_load_channel_props does nothing, and we either return channel, OR
in the error case we g_object_unref (channel) which frees channel.
The returned channel is either silently dropped (in the case where
it's already been set as the mask), or added to the image using
gimp_image_add_channel if not (which sinks the floating reference).
- xcf_load_channel_props encounters a single PROP_SELECTION. We create
a selection using gimp_selection_new (which again has a floating
reference), transfer ownership of the new selection to the image
using gimp_image_take_mask(), free the old channel, and finally set
channel to point to this new selection. Back in xcf_load_channel, IF
we hit the error case, we call g_object_unref (channel), which frees
the new selection - but we're still using it as the image's mask,
meaning we could eventually hit a use-after-free whenever someone
reads the mask.
- xcf_load_channel_props encounters 2 PROP_SELECTION's. After the first
PROP_SELECTION, channel is pointing to the image mask, which has
reference count == 1 (as explained above). When we hit the second
PROP_SELECTION: we create another new selection, followed by calling
gimp_image_take_mask() again. gimp_image_take_mask() call
g_object_unref() on the old mask, which frees it - but channel is still
pointing to this mask. We then call g_object_unref() on channel, which
is effectively a double-free.
We fix this by making sure to always ref_sink whatever object is put
into channel. gimp_image_take_mask also calls ref_sink, which means
we'll now bump the refcount up to 2 when the channel is being used as
the image's mask (and drop back to 1 if the mask is replaced, and down
to 0 when channel is unref'd).
See also ASAN output below from the 2x PROP_SELECTION scenario:
==6381==ERROR: AddressSanitizer: heap-use-after-free on address 0x6150000047d0 at pc 0x7fb5531ef31b bp 0x7ffe81e86cb0 sp 0x7ffe81e86ca8
READ of size 8 at 0x6150000047d0 thread T0
#0 0x7fb5531ef31a in g_type_check_instance_cast /home/ahunt/git/glib/_build/../gobject/gtype.c:4117:26
#1 0xb2346b in xcf_load_channel_props /home/ahunt/git/gimp/app/xcf/xcf-load.c:1742:41
#2 0xb1a3cc in xcf_load_channel /home/ahunt/git/gimp/app/xcf/xcf-load.c:2219:9
#3 0xb147eb in xcf_load_image /home/ahunt/git/gimp/app/xcf/xcf-load.c:653:17
#4 0xb121bb in xcf_load_stream /home/ahunt/git/gimp/app/xcf/xcf.c:305:19
#5 0x619ead in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/app/fuzzers/xcf_fuzzer.c:50:17
#6 0x51d414 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 0x507092 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 0x50d400 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 0x537452 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#10 0x7fb551e7c349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#11 0x4e0829 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120
0x6150000047d0 is located 336 bytes inside of 504-byte region [0x615000004680,0x615000004878)
freed by thread T0 here:
#0 0x5e8612 in free /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3
#1 0x7fb552d9ce08 in g_free /home/ahunt/git/glib/_build/../glib/gmem.c:199:3
#2 0x7fb552dc7a6b in g_slice_free1 /home/ahunt/git/glib/_build/../glib/gslice.c:1183:7
#3 0x7fb5531e7b04 in g_type_free_instance /home/ahunt/git/glib/_build/../gobject/gtype.c:2008:5
#4 0x7fb5531bfe3a in g_object_unref /home/ahunt/git/glib/_build/../gobject/gobject.c:3604:11
#5 0xd4d4d4 in gimp_image_take_mask /home/ahunt/git/gimp/app/core/gimpimage.c:3267:5
#6 0xb23438 in xcf_load_channel_props /home/ahunt/git/gimp/app/xcf/xcf-load.c:1739:13
#7 0xb1a3cc in xcf_load_channel /home/ahunt/git/gimp/app/xcf/xcf-load.c:2219:9
#8 0xb147eb in xcf_load_image /home/ahunt/git/gimp/app/xcf/xcf-load.c:653:17
#9 0xb121bb in xcf_load_stream /home/ahunt/git/gimp/app/xcf/xcf.c:305:19
#10 0x619ead in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/app/fuzzers/xcf_fuzzer.c:50:17
#11 0x51d414 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 0x507092 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 0x50d400 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 0x537452 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#15 0x7fb551e7c349 in __libc_start_main (/lib64/libc.so.6+0x24349)
previously allocated by thread T0 here:
#0 0x5e887d in malloc /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x7fb552d9ccf2 in g_malloc /home/ahunt/git/glib/_build/../glib/gmem.c:106:13
#2 0x7fb552dc72e0 in g_slice_alloc /home/ahunt/git/glib/_build/../glib/gslice.c:1072:11
#3 0x7fb552dc78ae in g_slice_alloc0 /home/ahunt/git/glib/_build/../glib/gslice.c:1098:18
#4 0x7fb5531e6e0a in g_type_create_instance /home/ahunt/git/glib/_build/../gobject/gtype.c:1911:17
#5 0x7fb5531c215e in g_object_new_internal /home/ahunt/git/glib/_build/../gobject/gobject.c:1945:24
#6 0x7fb5531c1d1f in g_object_new_valist /home/ahunt/git/glib/_build/../gobject/gobject.c:2288:16
#7 0x7fb5531c0e8b in g_object_new /home/ahunt/git/glib/_build/../gobject/gobject.c:1788:12
#8 0xdb7260 in gimp_item_new /home/ahunt/git/gimp/app/core/gimpitem.c:722:10
#9 0xce1668 in gimp_drawable_new /home/ahunt/git/gimp/app/core/gimpdrawable.c:1067:14
#10 0xe283e9 in gimp_selection_new /home/ahunt/git/gimp/app/core/gimpselection.c:626:13
#11 0xb2342a in xcf_load_channel_props /home/ahunt/git/gimp/app/xcf/xcf-load.c:1735:15
#12 0xb1a3cc in xcf_load_channel /home/ahunt/git/gimp/app/xcf/xcf-load.c:2219:9
#13 0xb147eb in xcf_load_image /home/ahunt/git/gimp/app/xcf/xcf-load.c:653:17
#14 0xb121bb in xcf_load_stream /home/ahunt/git/gimp/app/xcf/xcf.c:305:19
#15 0x619ead in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/app/fuzzers/xcf_fuzzer.c:50:17
#16 0x51d414 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 0x507092 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 0x50d400 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 0x537452 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#20 0x7fb551e7c349 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:
0x0c2a7fff88a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c2a7fff88b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa
0x0c2a7fff88c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fff88d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fff88e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c2a7fff88f0: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
0x0c2a7fff8900: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa
0x0c2a7fff8910: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2a7fff8920: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fff8930: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2a7fff8940: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
==6381==ABORTING
( crash-c35bcae86d35ce7d0cd8ffcb41a470f37354e018 )
diff --git a/app/xcf/xcf-load.c b/app/xcf/xcf-load.c
index 1fb0752832..3b1bcc872a 100644
--- a/app/xcf/xcf-load.c
+++ b/app/xcf/xcf-load.c
@@ -2176,6 +2176,28 @@ xcf_load_channel_props (XcfInfo *info,
continue;
}
+ if (*channel == gimp_image_get_mask (image))
+ {
+ /* PROP_SELECTION was already seen once for this
+ * channel. Let's silently ignore the second identical
+ * property to avoid a double free.
+ */
+ continue;
+ }
+ else if (gimp_image_get_mask (image) != NULL &&
+ ! gimp_channel_is_empty (gimp_image_get_mask (image)))
+ {
+ /* This would happen when PROP_SELECTION was already set
+ * on a previous channel. This is a minor case of data
+ * loss (we don't know which selection was the right one
+ * and we drop the non-first ones), and also means it's
+ * a broken XCF, though it's not a major bug either. So
+ * let's go with a stderr print.
+ */
+ g_printerr ("PROP_SELECTION property was set on 2 channels (skipping)\n");
+ continue;
+ }
+
/* We're going to delete *channel, Don't leave its pointer
* in @info. See bug #767873.
*/
