Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FBX] buffer overflow on loading FBX file #44376

Closed
RevoluPowered opened this issue Dec 14, 2020 · 3 comments
Closed

[FBX] buffer overflow on loading FBX file #44376

RevoluPowered opened this issue Dec 14, 2020 · 3 comments

Comments

@RevoluPowered
Copy link
Contributor

I still have buffer overflow in this PR

==336498==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6300071bfaa5 at pc 0x000003ece720 bp 0x7ffc4c302230 sp 0x7ffc4c302220
READ of size 1 at 0x6300071bfaa5 thread T0
    #0 0x3ece71f in FBXDocParser::Tokenize(std::vector<FBXDocParser::Token*, std::allocator<FBXDocParser::Token*> >&, char const*) modules/fbx/fbx_parser/FBXTokenizer.cpp:155
    #1 0x2c98395 in EditorSceneImporterFBX::import_scene(String const&, unsigned int, int, List<String, DefaultAllocator>*, Error*) modules/fbx/editor_scene_importer_fbx.cpp:131
    #2 0x7099000 in ResourceImporterScene::import(String const&, String const&, Map<StringName, Variant, Comparator<StringName>, DefaultAllocator> const&, List<String, DefaultAllocator>*, List<String, DefaultAllocator>*, Variant*) editor/import/resource_importer_scene.cpp:1320
    #3 0x5b7b268 in EditorFileSystem::_reimport_file(String const&) editor/editor_file_system.cpp:1798
    #4 0x5b86860 in EditorFileSystem::reimport_files(Vector<String> const&) editor/editor_file_system.cpp:1996
    #5 0x5b43586 in EditorFileSystem::_update_scan_actions() editor/editor_file_system.cpp:589
    #6 0x5b5d347 in EditorFileSystem::_notification(int) editor/editor_file_system.cpp:1174
    #7 0x5b969af in EditorFileSystem::_notificationv(int, bool) (/home/rafal/Pobrane/godot-fix-parser-crash-mesh-geometry/bin/godot.x11.tools.64s+0x5b969af)
    #8 0xe7f529b in Object::notification(int, bool) core/object.cpp:929
    #9 0x95e712d in SceneTree::_notify_group_pause(StringName const&, int) scene/main/scene_tree.cpp:985
    #10 0x95d79ef in SceneTree::idle(float) scene/main/scene_tree.cpp:525
    #11 0x1542726 in Main::iteration() main/main.cpp:2111
    #12 0x1434eb0 in OS_X11::run() platform/x11/os_x11.cpp:3608
    #13 0x13a2b56 in main platform/x11/godot_x11.cpp:56
    #14 0x7ff1592370b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #15 0x13a276d in _start (/home/rafal/Pobrane/godot-fix-parser-crash-mesh-geometry/bin/godot.x11.tools.64s+0x13a276d)

0x6300071bfaa5 is located 0 bytes to the right of 63141-byte region [0x6300071b0400,0x6300071bfaa5)
allocated by thread T0 here:
    #0 0x7ff15a6fc517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0xed2f31d in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:82
    #2 0x161b31a in PoolVector<unsigned char>::resize(int) core/pool_vector.h:582
    #3 0x2c97070 in EditorSceneImporterFBX::import_scene(String const&, unsigned int, int, List<String, DefaultAllocator>*, Error*) modules/fbx/editor_scene_importer_fbx.cpp:107
    #4 0x7099000 in ResourceImporterScene::import(String const&, String const&, Map<StringName, Variant, Comparator<StringName>, DefaultAllocator> const&, List<String, DefaultAllocator>*, List<String, DefaultAllocator>*, Variant*) editor/import/resource_importer_scene.cpp:1320
    #5 0x5b7b268 in EditorFileSystem::_reimport_file(String const&) editor/editor_file_system.cpp:1798
    #6 0x5b86860 in EditorFileSystem::reimport_files(Vector<String> const&) editor/editor_file_system.cpp:1996
    #7 0x5b43586 in EditorFileSystem::_update_scan_actions() editor/editor_file_system.cpp:589
    #8 0x5b5d347 in EditorFileSystem::_notification(int) editor/editor_file_system.cpp:1174
    #9 0x5b969af in EditorFileSystem::_notificationv(int, bool) (/home/rafal/Pobrane/godot-fix-parser-crash-mesh-geometry/bin/godot.x11.tools.64s+0x5b969af)
    #10 0xe7f529b in Object::notification(int, bool) core/object.cpp:929
    #11 0x95e712d in SceneTree::_notify_group_pause(StringName const&, int) scene/main/scene_tree.cpp:985
    #12 0x95d79ef in SceneTree::idle(float) scene/main/scene_tree.cpp:525
    #13 0x1542726 in Main::iteration() main/main.cpp:2111
    #14 0x1434eb0 in OS_X11::run() platform/x11/os_x11.cpp:3608
    #15 0x13a2b56 in main platform/x11/godot_x11.cpp:56
    #16 0x7ff1592370b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

Originally posted by @qarmin in #44371 (comment)

@RevoluPowered RevoluPowered self-assigned this Dec 14, 2020
@RevoluPowered RevoluPowered added this to the 3.2 milestone Dec 14, 2020
@qarmin
Copy link
Contributor

qarmin commented Dec 14, 2020

Probably this will be better project to test fbx imprort, because this zip contains all available fbx files from https://kenney.nl/assets

ALL_FBX_KENNEY.zip

@RevoluPowered
Copy link
Contributor Author

RevoluPowered commented Dec 14, 2020

Thanks! That is waaay better for providing full coverage :) @qarmin

        // buffer overflow is here apparently; line directly below
	for (const char *cur = input; *cur; column += (*cur == '\t' ? ASSIMP_FBX_TAB_WIDTH : 1), ++cur) {
		const char c = *cur;

		if (IsLineEnd(c)) {
			comment = false;

			column = 0;
			++line;
		}

		if (comment) {
			continue;
		}

		if (in_double_quotes) {
			if (c == '\"') {
				in_double_quotes = false;
				token_end = cur;

				ProcessDataToken(output_tokens, token_begin, token_end, line, column);
				pending_data_token = false;
			}
			continue;
		}

		switch (c) {
			case '\"':
				if (token_begin) {
					TokenizeError("unexpected double-quote", line, column);
				}
				token_begin = cur;
				in_double_quotes = true;
				continue;

			case ';':
				ProcessDataToken(output_tokens, token_begin, token_end, line, column);
				comment = true;
				continue;

			case '{':
				ProcessDataToken(output_tokens, token_begin, token_end, line, column);
				output_tokens.push_back(new_Token(cur, cur + 1, TokenType_OPEN_BRACKET, line, column));
				continue;

			case '}':
				ProcessDataToken(output_tokens, token_begin, token_end, line, column);
				output_tokens.push_back(new_Token(cur, cur + 1, TokenType_CLOSE_BRACKET, line, column));
				continue;

			case ',':
				if (pending_data_token) {
					ProcessDataToken(output_tokens, token_begin, token_end, line, column, TokenType_DATA, true);
				}
				output_tokens.push_back(new_Token(cur, cur + 1, TokenType_COMMA, line, column));
				continue;

			case ':':
				if (pending_data_token) {
					ProcessDataToken(output_tokens, token_begin, token_end, line, column, TokenType_KEY, true);
				} else {
					TokenizeError("unexpected colon", line, column);
				}
				continue;
		}

		if (IsSpaceOrNewLine(c)) {

			if (token_begin) {
				// peek ahead and check if the next token is a colon in which
				// case this counts as KEY token.
				TokenType type = TokenType_DATA;
				for (const char *peek = cur; *peek && IsSpaceOrNewLine(*peek); ++peek) {
					if (*peek == ':') {
						type = TokenType_KEY;
						cur = peek;
						break;
					}
				}

				ProcessDataToken(output_tokens, token_begin, token_end, line, column, type);
			}

			pending_data_token = false;
		} else {
			token_end = cur;
			if (!token_begin) {
				token_begin = cur;
			}

			pending_data_token = true;
		}
	}

RevoluPowered added a commit to RevoluPowered/godot that referenced this issue Dec 14, 2020
…verflow

Fixes:
- Element collection will only contain valid elements.
- Fixes buffer overflow in the FBX document
@akien-mga
Copy link
Member

Fixed by #44371.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants