Decoding RefCounted/Reference from binary (such as marshalls or networking) causes crash

[nathanfranke]

Godot version

3.3.3.stable
3.4.stable
v4.0.dev.custom_build [e734a7a] (2021-11-27)
v4.0.alpha.custom_build [d1dac84] (2022-06-22) (partially)

System information

Arch btw on 5.13.13-zen1-1-zen

Issue description

Put this code in any script:

extends Resource
class_name Data

export var best_engine: String

Put this code in _ready:

var data := Data.new()
data.best_engine = "Godot"
print(Marshalls.base64_to_variant(Marshalls.variant_to_base64(data, true), true))

On 3.3.3: crashes
On 3.4.4, 3.5.rc4: Prints [Object:0] on 3.4.4, 3.5.rc4 (segfaults if you access best_engine so definitely not expected)
On v4.0.dev.custom_build [e734a7a], crashes
On v4.0.alpha.custom_build [d1dac84], prints a different error Parser Error: Class "Data" hides a global script class. but is probably still reproducible past that.

Steps to reproduce

1. Run the MRP or use the above code.

Minimal reproduction project

Test.zip

---

Previous Reproduction (Networking)

Trying to send a resource to network peers.

I have set get_tree().multiplayer.allow_object_decoding = true

However, the client crashes when receiving the object.

Same thing happens with both NetworkedMultiplayerENet and WebSocket*

4.0 Valgrind Result

---

==26308== Invalid write of size 8
==26308==    at 0x6248E56: Object::set_script(Variant const&) (object.cpp:892)
==26308==    by 0x62467A2: Object::set(StringName const&, Variant const&, bool*) (object.cpp:417)
==26308==    by 0x5E77539: decode_variant(Variant&, unsigned char const*, int, int*, bool) (marshalls.cpp:600)
==26308==    by 0x5EF1B5E: MultiplayerAPI::decode_and_decompress_variant(Variant&, unsigned char const*, int, int*) (multiplayer_api.cpp:461)
==26308==    by 0x5F1DF89: RPCManager::_process_rpc(Node*, unsigned short, int, unsigned char const*, int, int) (rpc_manager.cpp:278)
==26308==    by 0x5F1D75B: RPCManager::process_rpc(int, unsigned char const*, int) (rpc_manager.cpp:224)
==26308==    by 0x5EF0536: MultiplayerAPI::_process_packet(int, unsigned char const*, int) (multiplayer_api.cpp:163)
==26308==    by 0x5EEF770: MultiplayerAPI::poll() (multiplayer_api.cpp:81)
==26308==    by 0x47963C3: SceneTree::process(double) (scene_tree.cpp:443)
==26308==    by 0x21EEB16: Main::iteration() (main.cpp:2544)
==26308==    by 0x21AD265: OS_LinuxBSD::run() (os_linuxbsd.cpp:342)
==26308==    by 0x21A9743: main (godot_linuxbsd.cpp:58)
==26308==  Address 0x10c65328 is 120 bytes inside a block of size 384 free'd
==26308==    at 0xA7CC18B: free (vg_replace_malloc.c:755)
==26308==    by 0x5D65151: Memory::free_static(void*, bool) (memory.cpp:168)
==26308==    by 0x223AB8F: void memdelete<RefCounted>(RefCounted*) (memory.h:112)
==26308==    by 0x5FACB2E: Variant::_clear_internal() (variant.cpp:1309)
==26308==    by 0x21AA822: Variant::clear() (variant.h:262)
==26308==    by 0x21AA847: Variant::~Variant() (variant.h:687)
==26308==    by 0x310B87E: GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (gdscript_vm.cpp:3318)
==26308==    by 0x2FFDD2D: GDScript::_super_implicit_constructor(GDScript*, GDScriptInstance*, Callable::CallError&) (gdscript.cpp:111)
==26308==    by 0x2FFDFC0: GDScript::_create_instance(Variant const**, int, Object*, bool, Callable::CallError&) (gdscript.cpp:137)
==26308==    by 0x2FFF7B7: GDScript::instance_create(Object*) (gdscript.cpp:371)
==26308==    by 0x6248E4E: Object::set_script(Variant const&) (object.cpp:892)
==26308==    by 0x62467A2: Object::set(StringName const&, Variant const&, bool*) (object.cpp:417)
==26308==  Block was alloc'd at
==26308==    at 0xA7C97C5: malloc (vg_replace_malloc.c:380)
==26308==    by 0x5D64A96: Memory::alloc_static(unsigned long, bool) (memory.cpp:75)
==26308==    by 0x5D64A1E: operator new(unsigned long, char const*) (memory.cpp:40)
==26308==    by 0x5D60AA3: Object* ClassDB::creator<Resource>() (class_db.h:151)
==26308==    by 0x622FB7F: ClassDB::instantiate(StringName const&) (class_db.cpp:545)
==26308==    by 0x5E772D5: decode_variant(Variant&, unsigned char const*, int, int*, bool) (marshalls.cpp:568)
==26308==    by 0x5EF1B5E: MultiplayerAPI::decode_and_decompress_variant(Variant&, unsigned char const*, int, int*) (multiplayer_api.cpp:461)
==26308==    by 0x5F1DF89: RPCManager::_process_rpc(Node*, unsigned short, int, unsigned char const*, int, int) (rpc_manager.cpp:278)
==26308==    by 0x5F1D75B: RPCManager::process_rpc(int, unsigned char const*, int) (rpc_manager.cpp:224)
==26308==    by 0x5EF0536: MultiplayerAPI::_process_packet(int, unsigned char const*, int) (multiplayer_api.cpp:163)
==26308==    by 0x5EEF770: MultiplayerAPI::poll() (multiplayer_api.cpp:81)
==26308==    by 0x47963C3: SceneTree::process(double) (scene_tree.cpp:443)

Steps to reproduce

1. Run the MRP with the --server cmdline flag.
2. Run the MRP without the --server cmdline flag (client).

Minimal reproduction project

3.x MRP: Test5.zip
4.x MRP: Test6.zip

[Calinou]

I can confirm this on 3.4beta. The client crashes when joining, but the server doesn't crash (it displays the client connection message only).

Client log with backtrace

Godot Engine v3.4.beta.custom_build.410598672 - https://godotengine.org
OpenGL ES 3.0 Renderer: NVIDIA GeForce GTX 1080/PCIe/SSE2
OpenGL ES Batching: ON
 
Joining
Connected: 1
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /lib64/libc.so.6(+0x3d320) [0x7f5afd44c320] (??:0)
[2] Map<StringName, GDScript::MemberInfo, Comparator<StringName>, DefaultAllocator>::find(StringName const&) (/home/hugo/Documents/Git/godotengine/godot/./core/map.h:518)
[3] GDScriptInstance::set(StringName const&, Variant const&) (/home/hugo/Documents/Git/godotengine/godot/modules/gdscript/gdscript.cpp:955)
[4] Object::set(StringName const&, Variant const&, bool*) (/home/hugo/Documents/Git/godotengine/godot/core/object.cpp:412)
[5] decode_variant(Variant&, unsigned char const*, int, int*, bool) (/home/hugo/Documents/Git/godotengine/godot/core/io/marshalls.cpp:444)
[6] MultiplayerAPI::_process_rpc(Node*, StringName const&, int, unsigned char const*, int, int) (/home/hugo/Documents/Git/godotengine/godot/core/io/multiplayer_api.cpp:307)
[7] MultiplayerAPI::_process_packet(int, unsigned char const*, int) (/home/hugo/Documents/Git/godotengine/godot/core/io/multiplayer_api.cpp:218)
[8] MultiplayerAPI::poll() (/home/hugo/Documents/Git/godotengine/godot/core/io/multiplayer_api.cpp:117)
[9] SceneTree::idle(float) (/home/hugo/Documents/Git/godotengine/godot/scene/main/scene_tree.cpp:?)
[10] Main::iteration() (/home/hugo/Documents/Git/godotengine/godot/main/main.cpp:2166)
[11] OS_X11::run() (/home/hugo/Documents/Git/godotengine/godot/platform/x11/os_x11.cpp:3640)
[12] /home/hugo/Documents/Git/godotengine/godot/bin/godot.x11.tools.64.llvm(main+0x16e) [0x2be621e] (/home/hugo/Documents/Git/godotengine/godot/platform/x11/godot_x11.cpp:55)
[13] /lib64/libc.so.6(__libc_start_main+0xd5) [0x7f5afd436b75] (??:0)
[14] /home/hugo/Documents/Git/godotengine/godot/bin/godot.x11.tools.64.llvm(_start+0x2e) [0x2be5fee] (??:?)
-- END OF BACKTRACE --
[1]    113664 IOT instruction (core dumped)  ~/Documents/Git/godotengine/godot/bin/godot.x11.tools.64.llvm

[qarmin]

Crash happens in this line

godot/core/object.cpp

Line 1020 in 95162ca

script_instance = s->instance_create(this);

==7267==ERROR: AddressSanitizer: heap-use-after-free on address 0x61400002a2c8 at pc 0x000011ceba68 bp 0x7ffd47a3ea00 sp 0x7ffd47a3e9f0
WRITE of size 8 at 0x61400002a2c8 thread T0
    #0 0x11ceba67 in Object::set_script(RefPtr const&) core/object.cpp:1020
    #1 0x11cd8fcd in Object::set(StringName const&, Variant const&, bool*) core/object.cpp:432
    #2 0x1263537e in decode_variant(Variant&, unsigned char const*, int, int*, bool) core/io/marshalls.cpp:444
    #3 0x12652f6b in MultiplayerAPI::_process_rpc(Node*, StringName const&, int, unsigned char const*, int, int) core/io/multiplayer_api.cpp:307
    #4 0x1264fca4 in MultiplayerAPI::_process_packet(int, unsigned char const*, int) core/io/multiplayer_api.cpp:218
    #5 0x1264af55 in MultiplayerAPI::poll() core/io/multiplayer_api.cpp:117
    #6 0xc5aaf92 in SceneTree::idle(float) scene/main/scene_tree.cpp:518
    #7 0x197d71d in Main::iteration() main/main.cpp:2166
    #8 0x1855a6c in OS_X11::run() platform/x11/os_x11.cpp:3640
    #9 0x17c0d8b in main platform/x11/godot_x11.cpp:55
    #10 0x7fecc8ae7564 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28564)
    #11 0x17c09ad in _start (/usr/bin/godots+0x17c09ad)

0x61400002a2c8 is located 136 bytes inside of 400-byte region [0x61400002a240,0x61400002a3d0)
freed by thread T0 here:
    #0 0x7fecc9a648f7 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x12244ef4 in Memory::free_static(void*, bool) core/os/memory.cpp:168
    #2 0x188c99d in void memdelete<Reference>(Reference*) core/os/memory.h:118
    #3 0x187d826 in Ref<Reference>::unref() core/reference.h:258
    #4 0x11db4224 in RefPtr::unref() core/ref_ptr.cpp:85
    #5 0x11f03fe2 in Variant::clear() core/variant.cpp:1008
    #6 0x17c126f in Variant::~Variant() core/variant.h:448
    #7 0x1e6db82 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) modules/gdscript/gdscript_function.cpp:263
    #8 0x1c6825e in GDScript::_create_instance(Variant const**, int, Object*, bool, Variant::CallError&) modules/gdscript/gdscript.cpp:102
    #9 0x1c6eab4 in GDScript::instance_create(Object*) modules/gdscript/gdscript.cpp:306
    #10 0x11ceb9ea in Object::set_script(RefPtr const&) core/object.cpp:1020
    #11 0x11cd8fcd in Object::set(StringName const&, Variant const&, bool*) core/object.cpp:432
    #12 0x1263537e in decode_variant(Variant&, unsigned char const*, int, int*, bool) core/io/marshalls.cpp:444
    #13 0x12652f6b in MultiplayerAPI::_process_rpc(Node*, StringName const&, int, unsigned char const*, int, int) core/io/multiplayer_api.cpp:307
    #14 0x1264fca4 in MultiplayerAPI::_process_packet(int, unsigned char const*, int) core/io/multiplayer_api.cpp:218
    #15 0x1264af55 in MultiplayerAPI::poll() core/io/multiplayer_api.cpp:117
    #16 0xc5aaf92 in SceneTree::idle(float) scene/main/scene_tree.cpp:518
    #17 0x197d71d in Main::iteration() main/main.cpp:2166
    #18 0x1855a6c in OS_X11::run() platform/x11/os_x11.cpp:3640
    #19 0x17c0d8b in main platform/x11/godot_x11.cpp:55
    #20 0x7fecc8ae7564 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28564)

previously allocated by thread T0 here:
    #0 0x7fecc9a64c47 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x12243eb5 in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:75
    #2 0x12243dc6 in operator new(unsigned long, char const*) core/os/memory.cpp:40
    #3 0x11df9eb4 in Object* ClassDB::creator<Resource>() core/class_db.h:140
    #4 0x11a6963a in ClassDB::instance(StringName const&) core/class_db.cpp:520
    #5 0x12634b56 in decode_variant(Variant&, unsigned char const*, int, int*, bool) core/io/marshalls.cpp:412
    #6 0x12652f6b in MultiplayerAPI::_process_rpc(Node*, StringName const&, int, unsigned char const*, int, int) core/io/multiplayer_api.cpp:307
    #7 0x1264fca4 in MultiplayerAPI::_process_packet(int, unsigned char const*, int) core/io/multiplayer_api.cpp:218
    #8 0x1264af55 in MultiplayerAPI::poll() core/io/multiplayer_api.cpp:117
    #9 0xc5aaf92 in SceneTree::idle(float) scene/main/scene_tree.cpp:518
    #10 0x197d71d in Main::iteration() main/main.cpp:2166
    #11 0x1855a6c in OS_X11::run() platform/x11/os_x11.cpp:3640
    #12 0x17c0d8b in main platform/x11/godot_x11.cpp:55
    #13 0x7fecc8ae7564 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28564)

SUMMARY: AddressSanitizer: heap-use-after-free core/object.cpp:1020 in Object::set_script(RefPtr const&)

[ELginas]

Tried running MRP on Windows 10 with 3.3.3.stable. Instead of crashing, client receives rpc with null parameter.

[nathanfranke]

In v4.0.alpha.custom_build [d1dac84], I am getting a different error (likely earlier in the chain):

Parser Error: Class "Data" hides a global script class.

This error is thrown during Marshalls.base64_to_variant. It doesn't happen when using ResourceLoader.load, which I would imagine goes through a similar execution.

[DaGamingWolf]

In v4.0.alpha.custom_build [d1dac84], I am getting a different error (likely earlier in the chain):

Parser Error: Class "Data" hides a global script class.

This error is thrown during Marshalls.base64_to_variant. It doesn't happen when using ResourceLoader.load, which I would imagine goes through a similar execution.

this may be related to the bug i found, if not the same thing. the same error pops up on my end in both godot 4 alpha 14 and alpha 15 as of this day. here's my own mrp, maybe it'll help track it down. on my side, ResourceLoader.load does cause this same error, and the console complains about not being able to load the resource.

Minimal reproducable project.zip

[dalexeev]

The issue seems fixed in 4.3 dev, probably by #90693 and/or #78219.

this may be related to the bug i found, if not the same thing.

See #65393. I'm not sure if this is a bug, since FLAG_BUNDLE_RESOURCES duplicates the script (like user://save.tres::GDScript_sx3gf). GDScript resources are not really intended for bundling. You cannot create an identical copy of a script, it will have a different FQCN.