Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 11 Nov 2015 11:49:51 +0100
From: Moritz Bechler <mbechler@...terphace.org>
To: oss-security@...ts.openwall.com
Subject: Re: Assign CVE for common-collections remote code
 execution on deserialisation flaw

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

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.