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

FileAccessPack::get_buffer moves the cursor past the end of file #85921

Closed
andreymal opened this issue Dec 8, 2023 · 0 comments · Fixed by #85991
Closed

FileAccessPack::get_buffer moves the cursor past the end of file #85921

andreymal opened this issue Dec 8, 2023 · 0 comments · Fixed by #85991
Milestone

Comments

@andreymal
Copy link

andreymal commented Dec 8, 2023

Tested versions

4.2.stable (exported only)

System information

Linux

Issue description

This line is the bug:

pos += p_length;

p_length is the buffer length provided by the user, but this line does not check if the result is bigger than the length of the file.

As a result, when user tries to get a buffer that is bigger than the file (something like file.get_buffer(99999999)), file.get_position() may be bigger than file.get_length().

It doesn't break Godot directly, but breaks any calculations based on get_position(). For example, it breaks relative seeking.

There is another variable t_read which is checked and should probably be used instead:

int64_t to_read = p_length;
if (to_read + pos > pf.size) {
eof = true;
to_read = (int64_t)pf.size - (int64_t)pos;
}

Steps to reproduce

Since this is a bug in FileAccessPack, it is only reproducible in exported projects (something like FileAccessUnix doesn't have this bug, so it can't be reproduced in the editor).

To reproduce, try to file.get_buffer(99999999) and then use file.get_position() for something.

Minimal reproduction project (MRP)

I made a simple script that attempts to make a relative seeking and breaks because of this bug:

FileAccessPackTest.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.

3 participants