Is HONK nothing sacred HONK? It's 2019 and an evil save file can pwn much-loved HONK Untitled Goose Game Fans of Untitled Goose Game should update their copy of the indie smash-hit following the discovery of a bug that can lead to malicious save files hijacking players' systems. Pulse Security bug-hunter Denis Andzakovic took credit for finding and responsibly disclosing the vulnerability, which does not appear to have been …
Seems many people misunderstand that advice.
"Be liberal in what you accept" means to assume you might be sent anything.
So you must deal with incomplete, partially corrupt and unexpected input appropriately, and should try to correct them where possible.
- ECC, checksums, sensible defaults, skip over unexpected input...
That's a fine sentiment, but it's pretty clear from context that Jon Postel intended the Interoperability Principle as a recommendation for the liberal interpretation of malformed data. That has frequently created security issues, as with, for example, permitting non-canonical UTF-8 sequences, which may bypass special-character blacklists but then be interpreted by parsers.
The Interoperability Principle was invaluable in promoting the spread of the Internet, TCP/IP, and many application protocols and languages (perhaps most notably HTTP and HTML). Over the past couple of decades it's increasingly become a liability.
How the heck can a save file run arbitrary code? Well, I haven't looked at this vuln in detail, but there's a known class of exploits that affects almost any framework that allows you to deserialize arbitrary classes, like PHP, C#, Java, Ruby, etc etc.
- Find a class in the program which does something in its Dispose() method (called when the object should release its resources), say the HonkBonk class.
- If the Dispose() method includes a callback, you're wide open, but there are several things you can exploit.
- There are a lot of .NET classes too, you can abuse those as well as the program's own classes.
- In your malicious save file you put a saved object for the HonkBonk class - for the callback field, put a lambda with your arbitrary code.
- Program tries to read the SaveData class from the save file
- Instead of the SaveData class, the BinaryFormatter sees a HonkBonk object - it creates it (it's a known class!) and reads the fields into it
- When the program tries to cast HonkBonk object to SaveData class, this fails, so you get a cast exception.
- The HonkBonk object is 'lost' (there are no references to it)
- The HonkBonk object gets garbage collected
- Dispose() is called on the HonkBonk object
- Your arbitrary code is executed
- * HONK*
You can use the SerializationBinder in .NET to stop it from attempting to handle completely arbitrary data.
Well, in terms of vulnerability risk assessment, as we do for CVSS scoring, you have to look at the threat model. A couple of mitigations:
* UGG is not a common application in the same sense as, say, Microsoft Word. The target population is not enormous.
* User interaction is required, and typically some social engineering. Users don't habitually open game-save files from untrusted sources. Again that limits targets, and complicates automated or bulk exploitation.
* Users should be running with normal privileges. Of course we know that users often run with excessive privileges, but in this case the vulnerable application doesn't have any good reason to do so, unlike with some targets.
* The target doesn't have direct access to sensitive data, so an exploit has to be chained with, or followed by, at least a pivot maneuver to get anything of value. That's not an obstruction for a determined attacker but it increases the attacker's work factor to do anything useful, and so discourages casual exploitation.
I still don't know that I'd rate this as "amusing", but it probably pushes it from Critical down to High or even Medium importance, depending on the user aspects of your threat model. (In my case, since I don't have to worry about any UGG users, it doesn't matter at all.)
That's an interesting exploit. Off the top of my head, I'd mitigate that sort of thing by:
Having your save data classes in their own namespace.
Make these plain classes that don't implement IDisposable (or your language specific equivalent)
Not having callbacks in destructor or dispose code (in C# at least, there's a well-known pattern for implementing IDisposable that allows for disposal chains without callbacks).
Type-checking the objects when deserialised to ensure they are the type you are expecting, and if possible enforcing a type in the deserialiser (i.e. Deserialiser.Deserialise<Goose>(payload)).
Make sure you have don't use polymorphism in your save data classes (i.e. don't have a Goose class that derives from a Bird class) - this mitigates the fact that if you wanted to call Deserialise<Bird> you might get a Goose object back, and this would be valid.
It sounds to me that this specific exploit comes about because it is using a library for (de)serialisation that allows configuration of the serialiser in the file itself (I've not read the technical details, but this sounds likely). The mistake here is to allow the file to override the default. To be honest, that's a bit of a security flaw in the library itself (at least allowing such behaviour by default). It doesn't surprise me that Microsoft would write it this way. I'm willing to bet you that the documentation for its behaviour is less than great as well.
allow code (lambdas) in a data file?
The problem is serializing the type information.
https://www.blackhat.com/docs/us-17/thursday/us-17-Munoz-Friday-The-13th-JSON-Attacks-wp.pdf is quite fond of using https://docs.microsoft.com/en-us/dotnet/api/system.windows.data.objectdataprovider.objectinstance?view=netframework-4.8
So... They instantiate an object of type ObjectDataProvider and this object happens to take a method name (and parameters) as a property value leaving only the task of binding to the ObjectDataProvider instance.
I was willing to give generic deserialization a pass, in case I was being a old fogey. ("Bah! Deserialization! Use proper file formats, you punks!")
But the verdict is bad as my instincts feared. You cannot include arbitrary data schema in the file format and let the software reconstruct the data according to that schema.
The classic presentation on the subject is "Marshalling [sic] Pickles from AppSecCali 2015. It's a good one; anyone interested in further detail on how this sort of thing is exploited in various languages and contexts might want to watch the video or at least browse the slides.
In the MITRE CWE scheme, it's CWE-502.
I immediately recalled this issue: https://www.theregister.co.uk/2013/01/10/ruby_on_rails_security_vuln/ Which inevitably lead to this one: https://www.theregister.co.uk/2013/05/30/rails_botnet_threat/
Deserialization of more-or-less arbitrary data CAN be handled securely, but you cannot be an idiot about it. In the case of Rails, the problem was that the standard Ruby YAML library co-oped the [] method. That's bad enough, but a) this was (and IS!) undocumented, and b) it could be triggered by any document. The maintainer of the library never accepted that this is a problem, so the last time I checked, YAML cannot be used in Ruby with standard libraries to load data from files that are not fully trusted if you have classes which do interesting things with [].
Both the library provider and the application developer have to do their jobs.
The Register 
Biting the hand that feeds IT
Copyright. All rights reserved © 1998–2024
