Executing Image.save_jpg_to_buffer function crashes Godot

[qarmin]

Tested versions

4.3.dev. 4728ff3

System information

Ubuntu 22.04 CI

Issue description

When executing (this code was automatically minimized, so it is possible, that an even more "minimal" project can be created)

extends Node
func _process(delta):
	var temp_variable26463 = CenterContainer.new()
	add_child(temp_variable26463)
	temp_variable26463.queue_free()
	var temp_variable26573 = Image.new()
	temp_variable26573.get_height()
	temp_variable26573.crop(17, 87)
	temp_variable26573.rgbe_to_srgb()
	temp_variable26573.compute_image_metrics(null, true)
	temp_variable26573.compress_from_channels(3, 77, 86)
	temp_variable26573.save_jpg_to_buffer(-46.5191543102264)

Godot crashes:

Godot Engine v4.3.dev.custom_build - https://godotengine.org

ERROR: Condition "format != FORMAT_RGBE9995" is true. Returning: Ref<Image>()
   at: rgbe_to_srgb (core/io/image.cpp:3620)
ERROR: Parameter "p_compared_image" is null.
   at: compute_image_metrics (core/io/image.cpp:4052)
ERROR: Cannot convert to <-> from compressed formats. Use compress() and decompress() instead.
   at: convert (core/io/image.cpp:525)
=================================================================
==18081==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61d0008014a0 at pc 0x563fa3943ea2 bp 0x7fff0e3ea630 sp 0x7fff0e3ea620
READ of size 1 at 0x61d0008014a0 thread T0
    #0 0x563fa3943ea1 in RGB_to_YCC thirdparty/jpeg-compressor/jpge.cpp:96
    #1 0x563fa395d1ff in jpge::jpeg_encoder::load_mcu(void const*) thirdparty/jpeg-compressor/jpge.cpp:861
    #2 0x563fa395eb8c in jpge::jpeg_encoder::process_scanline(void const*) thirdparty/jpeg-compressor/jpge.cpp:929
    #3 0x563fa3941aff in _jpgd_save_to_output_stream modules/jpg/image_loader_jpegd.cpp:177
    #4 0x563fa3941fdb in _jpgd_buffer_save_func modules/jpg/image_loader_jpegd.cpp:193
    #5 0x563fbbb5a051 in Image::save_jpg_to_buffer(float) const core/io/image.cpp:2546
    #6 0x563fbbc6f967 in void call_with_variant_args_retc_helper<__UnexistingClass, Vector<unsigned char>, float, 0ul>(__UnexistingClass*, Vector<unsigned char> (__UnexistingClass::*)(float) const, Variant const**, Variant&, Callable::CallError&, IndexSequence<0ul>) core/variant/binder_common.h:807
    #7 0x563fbbc549a8 in void call_with_variant_args_retc_dv<__UnexistingClass, Vector<unsigned char>, float>(__UnexistingClass*, Vector<unsigned char> (__UnexistingClass::*)(float) const, Variant const**, int, Variant&, Callable::CallError&, Vector<Variant> const&) core/variant/binder_common.h:568
    #8 0x563fbbc35894 in MethodBindTRC<Vector<unsigned char>, float>::call(Object*, Variant const**, int, Callable::CallError&) const core/object/method_bind.h:619
    #9 0x563fbd0b0ca5 in Object::callp(StringName const&, Variant const**, int, Callable::CallError&) core/object/object.cpp:837
    #10 0x563fbc4239da in Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) core/variant/variant_call.cpp:1212
    #11 0x563fa1d7471a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_vm.cpp:1734
    #12 0x563fa15ff2bc in GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) modules/gdscript/gdscript.cpp:1970
    #13 0x563faf700368 in bool Node::_gdvirtual__process_call<false>(double) scene/main/node.h:350
    #14 0x563faf62cf89 in Node::_notification(int) scene/main/node.cpp:57
    #15 0x563fa0c52a21 in Node::_notificationv(int, bool) scene/main/node.h:49
    #16 0x563fbd0b282e in Object::notification(int, bool) core/object/object.cpp:899
    #17 0x563faf83b7e4 in SceneTree::_process_group(SceneTree::ProcessGroup*, bool) scene/main/scene_tree.cpp:970
    #18 0x563faf83ea80 in SceneTree::_process(bool) scene/main/scene_tree.cpp:1047
    #19 0x563faf82d0a5 in SceneTree::process(double) scene/main/scene_tree.cpp:527
    #20 0x563fa029aed6 in Main::iteration() main/main.cpp:4018
    #21 0x563f9ff8ba0f in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:962
    #22 0x563f9ff69e73 in main platform/linuxbsd/godot_linuxbsd.cpp:85
    #23 0x7f9fe7429d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
    #24 0x7f9fe7429e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
    #25 0x563f9ff698f4 in _start (/home/runner/work/Qarminer/Qarminer/godot.linuxbsd.editor.dev.x86_64.san+0x3fe178f4)
0x61d0008014a0 is located 0 bytes to the right of 2080-byte region [0x61d000800c80,0x61d0008014a0)
allocated by thread T0 here:
    #0 0x7f9fe80b4887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x563fbb5b0dd3 in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:75
    #2 0x563fa011bdb3 in Error CowData<unsigned char>::resize<false>(long) core/templates/cowdata.h:340
    #3 0x563fa00f82c2 in Vector<unsigned char>::resize(long) core/templates/vector.h:95
    #4 0x563fa0e46fc4 in image_compress_cvtt(Image*, Image::UsedChannels) modules/cvtt/image_compress_cvtt.cpp:195
    #5 0x563fbbb5cc74 in Image::compress_from_channels(Image::CompressMode, Image::UsedChannels, Image::ASTCFormat) core/io/image.cpp:2663
    #6 0x563fbbc67deb in void call_with_variant_args_ret_helper<__UnexistingClass, Error, Image::CompressMode, Image::UsedChannels, Image::ASTCFormat, 0ul, 1ul, 2ul>(__UnexistingClass*, Error (__UnexistingClass::*)(Image::CompressMode, Image::UsedChannels, Image::ASTCFormat), Variant const**, Variant&, Callable::CallError&, IndexSequence<0ul, 1ul, 2ul>) core/variant/binder_common.h:756
    #7 0x563fbbc4e382 in void call_with_variant_args_ret_dv<__UnexistingClass, Error, Image::CompressMode, Image::UsedChannels, Image::ASTCFormat>(__UnexistingClass*, Error (__UnexistingClass::*)(Image::CompressMode, Image::UsedChannels, Image::ASTCFormat), Variant const**, int, Variant&, Callable::CallError&, Vector<Variant> const&) core/variant/binder_common.h:535
    #8 0x563fbbc27024 in MethodBindTR<Error, Image::CompressMode, Image::UsedChannels, Image::ASTCFormat>::call(Object*, Variant const**, int, Callable::CallError&) const core/object/method_bind.h:523
    #9 0x563fbd0b0ca5 in Object::callp(StringName const&, Variant const**, int, Callable::CallError&) core/object/object.cpp:837
    #10 0x563fbc4239da in Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) core/variant/variant_call.cpp:1212
    #11 0x563fa1d7471a in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_vm.cpp:1734
    #12 0x563fa15ff2bc in GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) modules/gdscript/gdscript.cpp:1970
    #13 0x563faf700368 in bool Node::_gdvirtual__process_call<false>(double) scene/main/node.h:350
    #14 0x563faf62cf89 in Node::_notification(int) scene/main/node.cpp:57
    #15 0x563fa0c52a21 in Node::_notificationv(int, bool) scene/main/node.h:49
    #16 0x563fbd0b282e in Object::notification(int, bool) core/object/object.cpp:899
    #17 0x563faf83b7e4 in SceneTree::_process_group(SceneTree::ProcessGroup*, bool) scene/main/scene_tree.cpp:970
    #18 0x563faf83ea80 in SceneTree::_process(bool) scene/main/scene_tree.cpp:1047
    #19 0x563faf82d0a5 in SceneTree::process(double) scene/main/scene_tree.cpp:527
    #20 0x563fa029aed6 in Main::iteration() main/main.cpp:4018
    #21 0x563f9ff8ba0f in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:962
    #22 0x563f9ff69e73 in main platform/linuxbsd/godot_linuxbsd.cpp:85
    #23 0x7f9fe7429d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
SUMMARY: AddressSanitizer: heap-buffer-overflow thirdparty/jpeg-compressor/jpge.cpp:96 in RGB_to_YCC
Shadow bytes around the buggy address:
  0x0c3a800f8240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a800f8250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a800f8260: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a800f8270: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a800f8280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3a800f8290: 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a800f82a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a800f82b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a800f82c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3a800f82d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3a800f82e0: 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
==18081==ABORTING

This example was found by Godot fuzzer - Qarminer, so it is quite unlikelly that this code could be used in real project, but still this should be handled gracefully.

Memory leaks or asan backtraces are visible when using Godot build with sanitizers support - https://github.com/qarmin/GodotBuilds/actions (linux -> linux-editor-sanitizers)

Steps to reproduce

Above

Minimal reproduction project (MRP)

Above

[brcontainer]

I tested your code and the crash did not occur, but in any case there are several incorrect uses of this code, follow the details:

1. Running this inside the _process(delta) doesn't seem correct. Working with images is a type of operation that consumes considerable resources, it is something that has to be worked on in an organized way, especially if it is something massive. Leaving such an operation loose within the "process" is likely to actually cause a problem, such as "buffer overflow"

2. -46.5191543102264 probably not an expected value, is expected between 0.1 and 1.0. Expected usage according to the documentation:

   PackedByteArray save_jpg_to_buffer(float quality=0.75 ) const

   Saves the image as a JPEG file to a byte array with the specified quality between 0.01 and 1.0 (inclusive)

3. You cannot use save_jpg_to_buffer(...) after using compress_from_channels(...). The save_jpg_to_buffer method expects an uncompressed image.

4. The use of rgbe_to_srgb() should only be done with images that use the RGBE9995 format, as you used "Image.new", the image is probably in the L8 format, so there is no way to use rgbe_to_srgb().

5. Another situation that does not seem normal is the use of compute_image_metrics(null, true), I have never used this method, but it seems to me that the value null is not something expected. Expected usage according to the documentation:

   Dictionary compute_image_metrics(Image compared_image, bool use_luma)

   Compute image metrics on the current image and the compared image.

6. Values of type "int" were used, when "enum" are expected in compress_from_channels(3, 77, 86) and even though numbers can be understood as specific enums, the values 77 and 86 are not part of the range accepted by each enum, CompressSource and ASTCFormat. Expected usage according to the documentation:

   Error compress_from_channels(mode: CompressMode, channels: UsedChannels, astc_format: ASTCFormat = 0)

Adjust the script to something within the expected usage, as described in the documentation and try to reproduce the crash, providing an MRP.

• https://docs.godotengine.org/en/stable/classes/class_image.html#class-image-method-save-jpg-to-buffer
• https://docs.godotengine.org/en/stable/classes/class_image.html#class-image-method-compute-image-metrics
• https://docs.godotengine.org/en/stable/classes/class_image.html#class-image-method-compress-from-channels
  • https://docs.godotengine.org/en/stable/classes/class_image.html#enum-image-compressmode
  • https://docs.godotengine.org/en/stable/classes/class_image.html#enum-image-usedchannels
  • https://docs.godotengine.org/en/stable/classes/class_image.html#enum-image-astcformat

[qarmin]

Did you tried to use builds with sanitizer? This builds should show same error as in first post.

The fuzzer's job is to find the smallest possible code to create a memory leak or crash(incorrectly used memory does not always cause problems, but this is seen much better with address sanitizer).

Usually this is code that users would never use in a released program, but it can happen that during testing they accidentally use something like this and this code can produce unexpected results.

[brcontainer]

If the idea is to look for failures caused by incorrect use of the code, by accident, then the correct thing is for the user to read the documentation or ask on a site like Stack Overflow (or even some AI site, eg.: ChatGPT) how to correctly use such a function, or ask for an example of correct use.

Does the crash occur if you use the functions as documented? As I described in the topics of my previous comment. Review the code and see if it still reproduces the issue. 👍

[qarmin]

Does the crash occur if you use the functions as documented? As I described in the topics of my previous comment. Review the code and see if it still reproduces the issue. 👍

For 99.9%, the error will not occur if even one line from the above code is changed.

The errors themselves in the logs obviously help the user to understand that they have made a mistake and should look at the documentation, but these errors should not cause the program to break memory or randomly shut down, which is what they do here and should be fixed because it is not the user's expectation of the program's operation.

[brcontainer]

Regardless of language, compilers/transpilers cannot predict or understand completely wrong usage, with incoherent values. They also cannot predict memory management problems, this is something beyond the language and the compiler, as there are things that can work to a certain level, but the person who manages or even kills/locks processes could be the operating system.

In any language you will see problems like this, when the user does random things, testing inconsistent values, and the compilers cannot fully predict. Ideally, you should use documentation when writing line by line, and not just when a failure occurs.

Some points to reinforce:

1. As I mentioned in the first comment, I tested the code exactly as you provided, the crash did not occur. The reason the crash doesn't occur is because it's probably a matter of my hardware being different from yours, or the OS managing memory differently, or my computer having more memory.

2. Problems of this type are not exactly bugs, they can be easily resolved by correcting the code, the Godot IDE itself has auto-complete+sugestions, like this:

   

3. VSCode also has some addons that can make it easier to use the language when writing code.

I hope I was able to guide you. I wish you good luck in your projects. 👍

[MewPurPur]

This is a crash report, qarmin doesn't need help. Godot should ideally never crash, even with erroneous inputs. The function should instead cleanly refuse to execute or something.

[fire]

#5 0x563fbbb5cc74 in Image::compress_from_channels(Image::CompressMode, Image::UsedChannels, Image::ASTCFormat) core/io/image.cpp:2663 might be the last image function to fail.