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

Add abstraction for ClassNameValidator, for default typing, @JsonTypeInfo #21

Closed
cowtowncoder opened this issue Dec 21, 2017 · 3 comments

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 21, 2017

Due to multiple CVEs for possible exploits via "default typing" (or, @JsonTypeInfo with base type of java.lang.Object) AND a small set of common "serialization gadgets", we have added blacklist of such gadgets. Blacklist is a moving target, with steady trickle of new types, and fundamentally can never be 100% complete.

Inclusion of black list is then problematic in a few ways:

  1. As protection it is unlikely to ever be complete for everybody, everywhere
  2. Since it's part of jackson-databind (for now at least), update requires new release, update by app/service developers
  3. Although there are ways around (2) (by subclassing), those are cumbersome and not well known

As to (3), it is worth noting that:

  • Subclassing BeanDeserializerFactory (used by databind itself for applying blacklist)
  • Custom Module (before DeserializerFactory is invoked)
  • Implementing custom com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver, even before deserializer, when resolving polymorphic type id as a class

can all be used for protection; but are either not module (sub-classing of deserializer factory), or otherwise cumbersome (module, type id resolver).

But it should be possible to take another approach: define a new small handler API, to be invoked when:

  1. Resolving subtype to serialize / deserialize
  2. Attempting to construct serializer / deserializer

This would allow many things, like:

  • Preventing deserialization and/or serialization of types deemed unsafe (or undesired?)
  • Changing of type on deserialization (or, for purpose of locating serializer, also serialization?)

one use of which is security check.

Callback(s) could then:

  • translate type as-is, for "that's fine"
  • substitute type (subtype for deserialization; supertype for serialization)
  • throw exception to indicate that type is illegal to serialize/deserialize

In theory it might even be possible to perhaps allow some special handling like "always deserialize as null"; but at this point this is speculation.

After defining such handler interface, it should be applied in two ways:

  1. For default typing, REQUIRE passing of such validator. While default implementation ("just block known serialiation gadgets") should be available, it SHOULD NOT be passed implicitly. Caller must pass it, to document their choice.
  2. For @JsonTypeInfo approach, a new property, with default choice, should suffice. Here defaulting should be ok since exploit only applies to case of java.lang.Object (and perhaps java.io.Serializable, java.util.Comparable?)
    • possibly require non-default choice for java.lang.Object et al?

and then polymorphic handler would need to be passed validator to use.

Net effect of this would be to allow finer-grained control of blocking certain types from polymorphic handling (esp. deserialization).

@jasinner
Copy link

jasinner commented Mar 2, 2018

I think we should also provide a way to supply a white list, in addition to a blacklist. Similar to the JEP-290 approach for the JDK.

@cowtowncoder
Copy link
Member Author

cowtowncoder commented Mar 2, 2018

@jasinner it might be possible to add a default implementation that could do that.

I am bit ambivalent on white-listing because it seems to go against the only real use case for default typing -- that is, open-ended "support anything" functionality -- and it seems to me that instead of white-listing, one should just use explicit base class other than java.lang.Object (or Serializable/Comparable), and/or type name and not class.
This is different from JDK serialization where there are fundamentally no limitations, and so white-listing is the only option.

Having said that I am not against adding functionality, or at very least making it easy to implement.

@cowtowncoder
Copy link
Member Author

Move to: FasterXML/jackson-databind#2195

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