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

Attempting to create an array of size 2147483648 and up crashes the whole game, not just the script. #7641

Closed
James103 opened this issue Jul 7, 2019 · 3 comments

Comments

@James103
Copy link
Contributor

@James103 James103 commented Jul 7, 2019

Version of OpenTTD

Crash logs are from 20190603-master-g846fc8fe09, but this bug has probably been in the game since NoAI/NoGo has been implemented.

Expected result

array(2147483648) and up should throw an error couldn't create/resize an array of/to size 2147483648, where the 2147483648 should be replaced with any number greater than or equal.

Actual result

array(2147483648) crashes the game in a number of different possible ways.

  • The game can close itself with no error message. This can create, or not create a crash log.
  • An OS system message "OpenTTD has stopped working" appears. No crash log is generated.
  • OpenTTD tries to allocate memory for the array (the amount allocated is the size times 12, mod 4294967296). If the truncated allocation succeeds, OpenTTD crashes. Otherwise, OpenTTD still crashes, but due to an out of memory error.

Crash log 1
Crash log 2
Crash log 3
Crash log 4

Steps to reproduce

In Start() of any AI or Game Script, put array(N) but replace N with the array size you want to test. Then run the script ingame. If N < 2147483648 and you have enough RAM, OpenTTD does not crash. Otherwise, OpenTTD will crash as described above.

Example code (main.nut):

class Test extends ScriptController // replace Script with AI or GS.
{
}

function Test::Start() {
    local x = array(N); // replace N with the array size you want.
}
@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Jul 7, 2019

It's difficult to safely limit Squirrel memory allocations without modifying the core Squirrel runtime. I did originally try to block allocations in #7516 if they broke the limit, but it turned out Squirrel did not handle failing allocations, and it made it impossible to recover from.

Are you testing with 32 bit or 64 bit builds? In 64 bit builds an allocations of 2**31 should technically be able to succeed, but I don't know if the allocator is happy with it still.

Regardless, I don't consider this a security issue at least, more of a "well don't do that then".

@James103
Copy link
Contributor Author

@James103 James103 commented Jul 7, 2019

All of my builds that I've used (since 2017) have been 32-bit. This is because I have a 32-bit system.

Why does this have to do with the allocator, even though the array size being at least 2**31 should already have been invalid? Other languages and compilers throw an error with array sizes greater than 2**31. Why not OpenTTD's version of Squirrel?

@LordAro
Copy link
Member

@LordAro LordAro commented Dec 31, 2019

Duplicate of #6322

@LordAro LordAro marked this as a duplicate of #6322 Dec 31, 2019
@LordAro LordAro closed this Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.