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

Godot serialization security issues (CVE-2018-1000224) #20558

Closed
Faless opened this issue Jul 29, 2018 · 2 comments
Closed

Godot serialization security issues (CVE-2018-1000224) #20558

Faless opened this issue Jul 29, 2018 · 2 comments

Comments

@Faless
Copy link
Collaborator

Faless commented Jul 29, 2018

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 ❤️

@akien-mga
Copy link
Member

Fixed in the master branch by feaf034 (will be included in 3.1-alpha1).
Fixed in the 3.0 branch by 5262d1b (included in 3.0.6-stable).
Fixed in the 2.1 branch by 497bc7d and c26094f (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 akien-mga changed the title Godot serialization security issues Godot serialization security issues (CVE-2018-1000224) Aug 20, 2018
@akien-mga
Copy link
Member

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

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

2 participants