godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.67k stars 20.11k forks source link

Heap buffer overflow when loading some SVG image #90371

Closed timothyqiu closed 3 months ago

timothyqiu commented 5 months ago

Tested versions

master[e5b4ef8e9522e950033cbece39a31a4a76da19c1]

System information

Arch Linux

Issue description

Open AssetLib in the editor, and address sanitizer decides to crash the editor.

ASan Output ``` ==4963==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5190024df0fd at pc 0x75ecfa4818df bp 0x75ecc75ff540 sp 0x75ecc75fece8 READ of size 1002 at 0x5190024df0fd thread T23 #0 0x75ecfa4818de in __interceptor_strlen /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:461 #1 0x55d2960687ca in tvg::strDuplicate(char const*, unsigned long) thirdparty/thorvg/src/common/tvgStr.cpp:219 #2 0x55d2960ab2d8 in simpleXmlParseCSSAttribute(char const*, unsigned int, char**, char**, char const**, unsigned int*) thirdparty/thorvg/src/loaders/svg/tvgXmlParser.cpp:564 #3 0x55d296087edc in _svgLoaderParserXmlCssStyle thirdparty/thorvg/src/loaders/svg/tvgSvgLoader.cpp:3164 #4 0x55d29608860e in _svgLoaderParser thirdparty/thorvg/src/loaders/svg/tvgSvgLoader.cpp:3208 #5 0x55d2960aa633 in simpleXmlParse(char const*, unsigned int, bool, bool (*)(void*, SimpleXMLType, char const*, unsigned int), void const*) thirdparty/thorvg/src/loaders/svg/tvgXmlParser.cpp:454 #6 0x55d29608ac01 in SvgLoader::run(unsigned int) thirdparty/thorvg/src/loaders/svg/tvgSvgLoader.cpp:3557 #7 0x55d296114394 in tvg::Task::operator()(unsigned int) thirdparty/thorvg/src/renderer/tvgTaskScheduler.h:64 #8 0x55d296115631 in tvg::TaskSchedulerImpl::run(unsigned int) thirdparty/thorvg/src/renderer/tvgTaskScheduler.cpp:153 #9 0x55d296114e66 in tvg::TaskSchedulerImpl::TaskSchedulerImpl(unsigned int)::{lambda()#1}::operator()() const thirdparty/thorvg/src/renderer/tvgTaskScheduler.cpp:120 #10 0x55d296116a57 in void std::__invoke_impl(std::__invoke_other, tvg::TaskSchedulerImpl::TaskSchedulerImpl(unsigned int)::{lambda()#1}&&) /usr/include/c++/13.2.1/bits/invoke.h:61 #11 0x55d296116a1a in std::__invoke_result::type std::__invoke(tvg::TaskSchedulerImpl::TaskSchedulerImpl(unsigned int)::{lambda()#1}&&) /usr/include/c++/13.2.1/bits/invoke.h:96 #12 0x55d2961169c7 in void std::thread::_Invoker >::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/include/c++/13.2.1/bits/std_thread.h:292 #13 0x55d29611699b in std::thread::_Invoker >::operator()() /usr/include/c++/13.2.1/bits/std_thread.h:299 #14 0x55d29611697f in std::thread::_State_impl > >::_M_run() /usr/include/c++/13.2.1/bits/std_thread.h:244 #15 0x55d2a023f0b2 in execute_native_thread_routine (/home/timothy/repos/godot-master/bin/godot.linuxbsd.editor.dev.x86_64.san+0x1524d0b2) (BuildId: 00782fbb3b377b7ae3d6d6a3814246024b24a92c) #16 0x75ecfa1bd559 (/usr/lib/libc.so.6+0x8b559) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) #17 0x75ecfa23aa3b (/usr/lib/libc.so.6+0x108a3b) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) 0x5190024df0fd is located 0 bytes after 1149-byte region [0x5190024dec80,0x5190024df0fd) allocated by thread T0 here: #0 0x75ecfa4e1359 in __interceptor_malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69 #1 0x55d29608cda1 in SvgLoader::open(char const*, unsigned int, bool) thirdparty/thorvg/src/loaders/svg/tvgSvgLoader.cpp:3675 #2 0x55d2960f995a in LoaderMgr::loader(char const*, unsigned int, std::__cxx11::basic_string, std::allocator > const&, bool) thirdparty/thorvg/src/renderer/tvgLoader.cpp:364 #3 0x55d29610c2d0 in tvg::Picture::Impl::load(char const*, unsigned int, std::__cxx11::basic_string, std::allocator > const&, bool) thirdparty/thorvg/src/renderer/tvgPicture.h:163 #4 0x55d29610b551 in tvg::Picture::load(char const*, unsigned int, std::__cxx11::basic_string, std::allocator > const&, bool) thirdparty/thorvg/src/renderer/tvgPicture.cpp:183 #5 0x55d29616c58b in ImageLoaderSVG::create_image_from_utf8_buffer(Ref, unsigned char const*, int, float, bool) modules/svg/image_loader_svg.cpp:85 #6 0x55d29616c158 in ImageLoaderSVG::load_mem_svg(unsigned char const*, int, float) modules/svg/image_loader_svg.cpp:74 #7 0x55d2983e7c57 in EditorAssetLibrary::_image_update(bool, bool, Vector const&, int) editor/plugins/asset_library_editor_plugin.cpp:852 #8 0x55d2983ebcc3 in EditorAssetLibrary::_request_image(ObjectID, int, String, EditorAssetLibrary::ImageType, int) editor/plugins/asset_library_editor_plugin.cpp:1025 #9 0x55d2983f51ed in EditorAssetLibrary::_http_request_completed(int, int, Vector const&, Vector const&) editor/plugins/asset_library_editor_plugin.cpp:1399 #10 0x55d29841ce83 in void call_with_variant_args_helper const&, Vector const&, 0ul, 1ul, 2ul, 3ul>(EditorAssetLibrary*, void (EditorAssetLibrary::*)(int, int, Vector const&, Vector const&), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul, 2ul, 3ul>) core/variant/binder_common.h:304 #11 0x55d29841aa02 in void call_with_variant_args const&, Vector const&>(EditorAssetLibrary*, void (EditorAssetLibrary::*)(int, int, Vector const&, Vector const&), Variant const**, int, Callable::CallError&) core/variant/binder_common.h:418 #12 0x55d298410d19 in CallableCustomMethodPointer const&, Vector const&>::call(Variant const**, int, Variant&, Callable::CallError&) const core/object/callable_method_pointer.h:103 #13 0x55d29ee1cf78 in Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const core/variant/callable.cpp:57 #14 0x55d29f65d745 in Object::emit_signalp(StringName const&, Variant const**, int) core/object/object.cpp:1201 #15 0x55d2995d9558 in Node::emit_signalp(StringName const&, Variant const**, int) scene/main/node.cpp:3904 #16 0x55d29951c169 in Error Object::emit_signal, Vector >(StringName const&, int, int, Vector, Vector) core/object/object.h:933 #17 0x55d299512cbe in HTTPRequest::_request_done(int, int, Vector const&, Vector const&) scene/main/http_request.cpp:483 #18 0x55d29952f6b8 in void call_with_variant_args_helper const&, Vector const&, 0ul, 1ul, 2ul, 3ul>(HTTPRequest*, void (HTTPRequest::*)(int, int, Vector const&, Vector const&), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul, 2ul, 3ul>) core/variant/binder_common.h:304 #19 0x55d29952b5d4 in void call_with_variant_args const&, Vector const&>(HTTPRequest*, void (HTTPRequest::*)(int, int, Vector const&, Vector const&), Variant const**, int, Callable::CallError&) core/variant/binder_common.h:418 #20 0x55d2995278f1 in CallableCustomMethodPointer const&, Vector const&>::call(Variant const**, int, Variant&, Callable::CallError&) const core/object/callable_method_pointer.h:103 #21 0x55d29ee1cf78 in Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const core/variant/callable.cpp:57 #22 0x55d29f644b09 in CallQueue::_call_function(Callable const&, Variant const*, int, bool) core/object/message_queue.cpp:221 #23 0x55d29f645e9d in CallQueue::flush() core/object/message_queue.cpp:326 #24 0x55d2996698f5 in SceneTree::process(double) scene/main/scene_tree.cpp:530 #25 0x55d293696cf9 in Main::iteration() main/main.cpp:4008 #26 0x55d2934aa66d in OS_LinuxBSD::run() platform/linuxbsd/os_linuxbsd.cpp:938 #27 0x55d293498876 in main platform/linuxbsd/godot_linuxbsd.cpp:85 #28 0x75ecfa157ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) Thread T23 created by T0 here: #0 0x75ecfa44a497 in __interceptor_pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:208 #1 0x55d2a023f199 in std::thread::_M_start_thread(std::unique_ptr >, void (*)()) (/home/timothy/repos/godot-master/bin/godot.linuxbsd.editor.dev.x86_64.san+0x1524d199) (BuildId: 00782fbb3b377b7ae3d6d6a3814246024b24a92c) #2 0x55d2961150a2 in tvg::TaskSchedulerImpl::TaskSchedulerImpl(unsigned int) thirdparty/thorvg/src/renderer/tvgTaskScheduler.cpp:120 #3 0x55d29611414f in tvg::TaskScheduler::init(unsigned int) thirdparty/thorvg/src/renderer/tvgTaskScheduler.cpp:201 #4 0x55d2960f8083 in tvg::Initializer::init(tvg::CanvasEngine, unsigned int) thirdparty/thorvg/src/renderer/tvgInitializer.cpp:129 #5 0x55d296171990 in initialize_svg_module(ModuleInitializationLevel) modules/svg/register_types.cpp:52 #6 0x55d2936e2da1 in initialize_modules(ModuleInitializationLevel) modules/register_module_types.gen.cpp:166 #7 0x55d293686915 in Main::setup2() main/main.cpp:3035 #8 0x55d29367c1ad in Main::setup(char const*, int, char**, bool) main/main.cpp:2404 #9 0x55d29349881c in main platform/linuxbsd/godot_linuxbsd.cpp:74 #10 0x75ecfa157ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:461 in __interceptor_strlen Shadow bytes around the buggy address: 0x5190024dee00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024dee80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024def00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024def80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024df000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x5190024df080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00[05] 0x5190024df100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x5190024df180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024df200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024df280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5190024df300: 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 ==4963==ABORTING ```

Steps to reproduce

Make sure your editor build has Address Sanitizer enabled.

Open AssetLib tab inside the editor, search for "Remote Transform" and wait.

Or extract and open the MRP with the editor.

Minimal reproduction project (MRP)

svg_heap_buffer_overflow.zip

fire commented 5 months ago

Do you know if there's more information about the svg I can pass to the thorvg folks @hermet

timothyqiu commented 4 months ago

Narrowed down to the icon of this plugin.

Extract svg_heap_buffer_overflow.zip. Open the project in a sanitizer build crashes with heap-buffer-overflow during the asset import process.


Edit: Narrowed down further. Save the following script as test.gd.

The issue can be reproduced by executing godot --headless -s test.gd.

test.gd ```gdscript extends SceneTree const SVG_STRING := """ """ func _initialize() -> void: print(">> Start") var image := Image.new() print(image.load_svg_from_string(SVG_STRING)) print(">> Done") quit() ```
hermet commented 4 months ago

@fire thorvg svg maintainers: @JSUYA @mgrudzinska We will check our side as well, thanks.

JSUYA commented 4 months ago

Hi I tested the attached svg images(icon.svg, RemoteTransformControl.svg) with version 0.12.10 (e69d63688b) of thorvg, but was unable to reproduce the problem. Recently, various issues in the svg loader module have been fixed. Can you check whether the same problem occurs in the latest version of thorvg?

timothyqiu commented 4 months ago

@JSUYA Hi

In the parser code, I find thorvg uses lots of string functions like strlen and strchr.

However, SvgLoader::open() accepts a buffer pointer & size, it's not necessarily null terminated.

https://github.com/godotengine/godot/blob/029aadef563fb69cf49aa9795b62f27171f8c3f4/thirdparty/thorvg/src/loaders/svg/tvgSvgLoader.cpp#L3697-L3711

Making the function allocate size + 1 and put null at content[size] fixes the problem :tada:

hermet commented 4 months ago

Hello, @JSUYA has handled the issue. The fix will be applied in thorvg v0.13.1. thanks.