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

Variant databind artifact that doesn't support default typing #41

Open
johngmyers opened this issue Jun 26, 2019 · 9 comments
Open

Variant databind artifact that doesn't support default typing #41

johngmyers opened this issue Jun 26, 2019 · 9 comments

Comments

@johngmyers
Copy link

I would like a variant of databind that doesn't support the "dynamic typing" anti-feature.

Every so often a new class gets added to the "default typing" blacklist. Whenever this happens, security scanners report all projects using databind as having a critical vulnerability and requiring "remediation" in a short timeframe.

While it is extremely unlikely that any of the projects are actually using default typing, the scanners can't figure this out. (If any projects actually did enable it, that would be a significant security concern in itself.)

A separate databind variant that doesn't support default typing would permit scanners to determine these blacklist additions are not relevant and thus avoid this periodic waste of effort.

Alternatively, the blacklist could be moved to a new, separate artifact. Databind would then refuse to enable default typing unless it could load that artifact.

@cowtowncoder
Copy link
Member

Work closely related to this would be:

FasterXML/jackson-databind#2195

which will be part of 2.10.

Major challenge for 2.x is unfortunately backwards compatibility: while it is possible to add stricter mechanisms that do not provide easy block-listing approach (although one can obviously implement such), it is not legal (wrt Semantic versioning for public API) to remove existing method.
3.0 will remove existing inherently unsafe configuration options and only support default typing when explicit validator is passed.

@johngmyers
Copy link
Author

My suggestion is to do this with dependencies. Have jackson-databind add a dependency to a new package, say "jackson-defaulttyping". Then a client could explicitly exclude that dependency, causing calls to the existing method/option to throw an exception.

@cowtowncoder
Copy link
Member

Ok. I am not sure how that would exactly help with 2.x (and 3.x would not need it), wrt backwards compatibility -- if users would have to change actual Maven dependency to new package, and would require significant refactoring of ObjectMapper.
And if "no change" case started throwing exceptions, that would be violating versioning expectations directly.

@johngmyers
Copy link
Author

Users would not have to change the actual Maven dependency. jackson-databind would list in its pom file a dependency on the new jackson-defaulttyping, so that would be pulled in by default. If users then added an explicit exclusion of that jackson-defaulttyping dependency, only then would they would get the exceptions.

With the blacklist in jackson-defaulttyping, the CVEs would be against that package and the users excluding it would not get the scanner hits.

@cowtowncoder
Copy link
Member

Ok. I don't think I see this working in a way that I would to pursue it: there'd be yet another artifact, complexity for use and so on. All this just to work around inadequacies of security check software.

@MarkusKull
Copy link

In our build environment, security scanners complained each time whenever a new gadget was found. Quite a chore. This even although we dont use polymorphic deserialization aka default typing. We deem polymorphic deserialization an anti-feature which has already precluded the use of one library (zalando problem). If SpringBoot supports other json implementations, we may drop jackson.

Not sure whether a to-be-excluded artifact would help: Most scanners are dumb and flag any potentially harmful functionality, even when not enabled. It is doubtful all scanners would detect the exclusion.
A better idea would be to move any functionality regarding default typing into a new artifact and jackson-module. Users would have to explicitly depend onto this artifact to enable default-typing. This is of course an idea for jackson3 only.

@cowtowncoder
Copy link
Member

There is one other idea that I think works better, and I'll send a proposal to mailing list: deprecated and document old methods; for future CVEs do not mark 2.10 as vulnerable because:

  1. Functionality is not enabled by default (true actually for 2.9, but that ship has sailed)
  2. To enable functionality, user is calling deprecated functionality with explicit warnings in documentation regarding security concerns.

To me this makes most sense.

As to 3.x same applies except for the fact that there are no methods that would enable unsafe behavior without user explicitly passing implementation that does no validation; Jackson will not offer such implementation (nor deny-list based alternative).

@cowtowncoder
Copy link
Member

Actually, thinking about this a bit more, I think I agree with your idea for 3.x -- default typing setting could be offered via Module, and NOT as default feature of databind. All supporting functionality is within databind, but no need to offer easy enabling.

Could you file an RFE issue for jackson-databind for such refactoring please?

@MarkusKull
Copy link

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

3 participants