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 PolymorphicTypeValidator, for limiting subtypes allowed by default typing, @JsonTypeInfo #2195

Closed
cowtowncoder opened this issue Dec 6, 2018 · 10 comments
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 6, 2018

(note: moved from FasterXML/jackson3-dev#21)

So: there are many CVEs that exploit permissive nature of class-name-based polymorphic deserialization, and especially so-called "default typing", in which polymorphic handling is applied without annotations and with base type of java.lang.Object.
Since set of potentially dangerous class types to exploit ("gadget types") is unbounded, relying on block-listing (which Jackson does) is not effective in long term. And although there are mechanisms one can use to limit accessibility they are cumbersome and not well documented (Spring framework has built upon this, however, kudos to their persistence and ingenuity :) ).

But providing for abstraction that could allow simple strategies such as allow-listing (i.e. enumerating allowed classes), or simple predicate, should not be difficult.
Now that we are planning to do one more 2.x release, 2.10, with easier configurability (via ObjectMapper.Builder), we should consider adding an extension point, instead of waiting until 3.0 which was the original plan.


From original issue, some further thoughts:

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

  • Resolving subtype to serialize / deserialize
  • 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.

As to applicability: although we should not make definition of such handler mandatory for 2.x,


Some examples of usage can be found from unit tests src/test/java/com/fasterxml/jackson/databind/jsontype/vld/ValidatePolymBaseTypeTest.java, src/test/java/com/fasterxml/jackson/databind/jsontype/vld/ValidatePolymSubTypeTest.java and ``src/test/java/com/fasterxml/jackson/databind/jsontype/vld/BasicPTVTest.java`

but here is an example:

PolymorphicTypeValidator validator = ...; // build validator
ObjectMapper saferMapper = JsonMapper.builder()
   .activateDefaultTyping(validator) // note the new name of method
   .build();
String json = mapper.writeValueAsString(myDefaultTypeUsingValue);

where validator could be complete custom one, or, instance of BasicPolymorphicTypeValidator like so:

        final PolymorphicTypeValidator validator = BasicPolymorphicTypeValidator.builder()
                .allowIfBaseType(MyBaseType.class)
                .build();

which would then allow all subtypes of MyBaseType, which is something user has control over (and thereby typically has no unsafe subtypes -- and definitely no widely shared ones)


EDIT 20-Aug-2019: New methods in ObjectMapper (and MapperBuilder) use name activateDefaultTyping() over old now-deprecated enableDefaultTyping() (and similarly for disable/deactivate). This to make it easier to code-search for old methods without needing argument checks.


EDIT 16-Oct-2019: Further validating this approach a good overview by Sonatype security research team: https://blog.sonatype.com/jackson-databind-the-end-of-the-blacklist

@cowtowncoder
Copy link
Member Author

Ok. So, it looks like one obvious injection point is

  • within TypeIdResolver.typeFromId()
  • for types ClassNameIdResolver (and MinimalClassNameIdResolver but that's a sub-class)

so that would allow handling of dangerous type at an earlier point.
We could also then pass _baseType to give bit more context.

I also think that name SubTypeValidator could be reused in 2.10, as long as we move it to a new package.

Some open questions:

  1. Should we immediately add validation for serialization too? (no current use cases, but in theory it could also be dangerous to serialize certain polymorphic types)
  2. Is there need to also validate that a deserializer should be created for certain type; beyond validation at type resolution? (no current use case)

@cowtowncoder
Copy link
Member Author

I have working implementation in 2.10 (and merged into master for 3.0 as well), with core abstraction of PolymorphicTypeValidator. Different validator for annotated (@JsonTypeInfo) and default typing:

  1. For annotated, either use JsonMapper.builder().polymorphicTypeValidator(ptv) (new in 2.10) or ObjectMapper.setPolymorphicTypeValidator(ptv)
  2. For default typing, pass new 1st argument for various enableDefaultTyping() methods (ObjectMapper or JsonMapper.builder())

I will be tweaking details a little bit still, and add standard BasicPolymorphicTypeValidator for users to configure allow-list approach.

@melloware
Copy link

Can't wait to get this fix in 2.10 so my company CVE alarm stops going on for every one of my builds. Sonatype IQ server reports this every time. Any estimate on a 2.10 release with this fix in it?

@cowtowncoder
Copy link
Member Author

@melloware yes, I feel your pain. This is why this is #1 priority for 2.10.

I am still hoping to get 2.10.0.pr1 out within couple of weeks -- most likely by early-/mid-June 2019 now.

@cowtowncoder cowtowncoder added this to the 2.10.0 milestone May 31, 2019
@cowtowncoder cowtowncoder changed the title Add abstraction for ClassNameValidator, for default typing, @JsonTypeInfo Add abstraction PolymorphicTypeValidator, for limiting subtypes allowed by default typing, @JsonTypeInfo May 31, 2019
@cowtowncoder
Copy link
Member Author

cowtowncoder commented May 31, 2019

Ok. So, the name of the handler used is PolymorphicTypeValidator, and there is one public convenience implementation, BasicPolymorphicTypeValidator with builder-style construction mechanism. Mechanism are allow "opt-in" (allow-match), with one exception of allowing blocking of specific base type (often java.lang.Object). For example:

  • BasicPolymorphicTypeValidator.builder().allowIfBaseType("com.fasterxml.").build()
    • would allow all values of polymorphic properties where declared base type is in com.fasterxml package
  • BasicPolymorphicTypeValidator.builder().allowIfSubType(MyType.class).build()
    • would allow all polymorphic values that are MyType or its sub-class

and so on.

@cowtowncoder
Copy link
Member Author

Quick note: see #2428 for small renaming -- new methods will use "activate"/"deactivate" over "enable"/"disable", so:

    mapper.activateDefaultTyping(...)

    // or

    ObjectMapper mapper JsonMapper.builder()
       .activateDefaultTyping(validator, ...)
       .build();

@cowtowncoder
Copy link
Member Author

Note to anyone watching this issue: Jackson 2.10.0 was released and is now available via Maven Central.

@retrodaredevil
Copy link

Just thought I'd share that I updated the wiki page on this: https://github.com/FasterXML/jackson-docs/wiki/JacksonPolymorphicDeserialization

I left the out of date part at the top. I'm not sure if the entire page is up to date as I just saw the inconsistency in using enableDefaultTyping and changed it to activateDefaultTyping.

@cowtowncoder
Copy link
Member Author

@retrodaredevil Thank you -- one thing worth noting would be that activateDefaultTyping() was added in 2.10, so will not work with older versions. No need to mention deprecated method, as that can be seen from 2.10 javadocs.

@KartikChugh
Copy link

Removed the out of date warning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants