oss-sec mailing list archives

Re: Assign CVE for common-collections remote code execution on deserialisation flaw


From: Moritz Bechler <mbechler () eenterphace org>
Date: Wed, 11 Nov 2015 11:49:51 +0100

Hi,

1. Most vulnerabilities require a "source" of untrusted data, and a 
"sink" in the code where that data is used unsafely.  In my
experience, most vulnerabilities are best corrected by making the
"sink" safe.  For example: output encoding in HTML or prepared
statements in SQL.  While many people preach input validation (or
*gag* "sanitization"), this is not how most classes of bugs are fixed
reliably.
The problem here is that the amount of potential "sinks" is incredibly
large - everything on your classpath. No objection here to hardening the
specific instance, but thinking one will be safe afterwards is a
misconception. Like someone else put it, it's a game of whack-a-mole.

2. There's absolutely no reason serialization can't be done safely.
It's absurd to think otherwise, given the fact that developers
regularly serialize/marshal/pickle objects into a wide variety of
formats, accept these from untrusted sources, deserialize them and
aren't made vulnerable by it.  Examples: JSON from websites, XML in
SOAP, ...  Any advice telling developers that "you shouldn't
deserialized objects from untrusted sources" must apply only to
software that has a flawed deserialization design.
Sure, as long as nobody does any funny stuff in their default
constructors, setters or getters. But the difference there is that these
unmarshallers (at least the ones I know of) only act on a very specific
set of classes you mostly control.
Looking at the flawed deserialization design at hand...

3. Java *does* provide a way to distinguish those to be trusted during
deserialization from all other classes.  Trusted classes implement
Serializable.  If you implement Serializable, you've been added to a
white list of code that should be safe.  If you've added a "sink" to
your Serializable objects, then that's a vulnerability.  Not only is
it a "sink", but validating the input prior to deserialization is
nearly impossible, which means you can't filter it at the "source"
even if you wanted to.  
And that's the assumption you are making. There is no such statement in
the Serializable definition, neither is anywhere defined what is
acceptable behavior for a readObject method and neither forbids the
collection API to do something dangerous in a getter (which in OpenJDK
seems to generally be an acceptable call).
Serializable is inherited, so the base class can give guarantees about
it's children - don't think so.

Serialization is also used for passivation in trusted contexts where
different rules apply and checking all this code is an unnecessary
effort. In fact I would guess that the majority of the classes out there
are solely Serializable for that purpose. These concepts don't mix well
and the mixture unnecessarily increases the attack surface.


So I do agree with you that Oracle could do a much better job here.
They could give us better tools and better ways to whitelist the kinds
of objects we're willing to accept.  Good luck convincing them of
that.  Last I checked, they still think XMLDecoder is ok.


Maybe forcing them to take a position on the RMI implementation will be
of some use. They clearly assume deserialization is safe there and fun
fact, even use it for the authentication crendentials.


Moritz


Current thread: