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

Corrupted compressed file can be read without errors in GDscript #57049

Closed
mbrlabs opened this issue Jan 21, 2022 · 0 comments · Fixed by #58278
Closed

Corrupted compressed file can be read without errors in GDscript #57049

mbrlabs opened this issue Jan 21, 2022 · 0 comments · Fixed by #58278

Comments

@mbrlabs
Copy link
Contributor

mbrlabs commented Jan 21, 2022

Godot version

3.4.2

System information

Windows 11

Issue description

When writing a compressed file with Godot and then intentionally modifying it using a hex editor, the file can still be opened by Godot and does not return an error in GDscript (file.open_compressed(OUTPUT_FILE, File.READ, COMPRESSION) returns OK). It still prints an error though (can't be detected AFAIK in GDscript). When the corupted part of the file is farer away from the beginning of the file (i presume because it does not fit into the intial buffer), the error is printed as well when calling one of the get_* methods. But get_error() is always OK.

This is the error:

E 0:00:20.310   decompress: Condition "err != 1" is true. Returned: -1
  <C++ Source>  core/io/compression.cpp:166 @ decompress()
  <Stack Trace> Main.gd:22 @ _on_ReadButton_pressed()

The return value of decompress method is dicarded in all instances when it is called in file_access_compressed.cpp. For example:

if (read_block < read_block_count) {
//read another block of compressed data
f->get_buffer(comp_buffer.ptrw(), read_blocks[read_block].csize);
Compression::decompress(buffer.ptrw(), read_blocks.size() == 1 ? read_total : block_size, comp_buffer.ptr(), read_blocks[read_block].csize, cmode);
read_block_size = read_block == read_block_count - 1 ? read_total % block_size : block_size;
read_pos = 0;

Shouldn't this return ERR_FILE_CORRUPT when decompress() returns -1 so that the user knows the file is corrupt?

Steps to reproduce

  1. Write a compressed file (compression method does not matter i think) using Godot
var file := File.new()
var res := file.open_compressed(OUTPUT_FILE, File.WRITE, File.COMPRESSION_GZIP)
if res != OK:
	printerr("Failed to open file for writing")
	return

file.store_string("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua")
file.close()
print("File written!")
  1. Open the file in a Hex Editor and change something (not the magic number at the start)
  2. Read the file using Godot again
var file := File.new()
var res := file.open_compressed(OUTPUT_FILE, File.READ, File.COMPRESSION_GZIP)
if res != OK:
	printerr("Failed to open file for reading")
	return
print(file.get_error())
	
file.get_buffer(file.get_len())
print(file.get_error())
file.close()

file.get_error() and file.open_compressed() will not report errors. But you can see some errors in the Debugger.

Minimal reproduction project

FileCompressionBug.zip

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

Successfully merging a pull request may close this issue.

2 participants