Godot: Godot serialization security issues (CVE-2018-1000224)

[Faless]

THIS ISSUE IS ALREADY FIXED. IT IS TEMPORARILY LEFT OPEN TO RAISE AWARENESS

Godot 2.1.5

Godot 3.0.6

Godot version: Any version of Godot since I could trace (2014, when it was open sourced)

OS/device including version: Any

Issue description:

Few weeks ago, I noticed by chance that passing a specific array of bytes to bytes2var() crashed the engine.

(in 3.0 bytes2var(PoolByteArray([20, 0, 0, 0, 0, 0, 0, 255]))).

After further investigations I realised that the (de)serialization code hadn't seen love in a while, so I went for a full audit and found out there were actually a few more issues.

The main issues where:

• Crash due to unbound allocation: Array types sometimes didn't have the right size check (due to incorrenct signed/unsigned comparison), causing the engine to try to allocate huge ammount of memory and being terminated by the OS.
• Read buffer overflow: Some types had incorrect size checks potentially causing buffer overflow during reading.
• Leak of memory content: Some strings padding where not correctly zeroized, potentially causing freed-but-not-cleared internal memory disclosure over network (although be reassured by the fact that crypto libs like OpenSSL and mbedTLS does clear their memory when freeing things like private key and so on).

I decided to privately contact few other developers (@reduz, @akien-mga, @hpvb, @karroffel) and we agreed to embargo this issue while we worked on a fix.

If you are reading this, patches are are already available (so this issue is actually closed), and updated binaries are available for download at the official Godot website.

Steps to reproduce:
Run a script that executes (3.0):

bytes2var(PoolByteArray([20, 0, 0, 0, 0, 0, 0, 255]))

Minimal reproduction project:

Please see this testing project I made to perform automatic tests.

Afterwords
First, I would like to thank all the devs involved, who helped me a lot during the development of the fix, and stayed online on weekends to do the releases.

In my humble opinion, this incident, along with the history of this engine, clearly shows how source code availability and great communities like this one, can improve software quality with concrete benefits to everyone.

From a security prospective, responsible disclosure is, in my opinion, the best road to security. In this sense, in the coming month we will be working on setting up guidelines and procedures in case anyone discovers a new security issue.

In the meantime, I invite everyone with a passion in security, to try and break our engine, the network code, the serialization process. If you find anything, let us know here on Github or on IRC at #godotengine-devel , we won't sue you, we will actually thank you :heart:

[akien-mga]

Fixed in the master branch by feaf03421dd (will be included in 3.1-alpha1).
Fixed in the 3.0 branch by 5262d1bbc (included in 3.0.6-stable).
Fixed in the 2.1 branch by 497bc7d5fd and c26094fd8 (included in 2.1.5-stable).

Thanks a lot @Faless for tracking down those issues, fixing them and disclosing them privately so that we can work on updating all supported branches without putting Godot-based networked games at risk.

You've earned a couple beers at next GodotCon and a cookie--pending acceptance of our cookie banner according to GDPR of course!

[akien-mga]

For the reference, this was assigned CVE-2018-1000224.