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

Trim tokens in error messages to 256 byte to prevent attacks #322

Merged
merged 2 commits into from
Jan 12, 2017

Conversation

asoldano
Copy link
Contributor

@asoldano asoldano commented Oct 6, 2016

See https://issues.jboss.org/browse/JBEAP-6316 (description and reproducer)
The fix here is really minimal, it might make sense to have something configurable, etc.

@cowtowncoder
Copy link
Member

Makes sense. Couple of things that would be good to improve

  1. Unit test to verify (and guard against regression)
  2. Support for character-backed (ReaderBasedJsonParser) reader too

I am ok with 256 character fixed limit itself.

Aside from that, I'd be happy to merge.
One practical thing needed is filled CLA from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and usually easiest way is to print, fill & sign, scan and email to info at fasterxml dot com.
We need this just once (assuming we haven't asked for once yet?).

@richardfontana
Copy link

I'm a lawyer for @asoldano's employer, Red Hat.

The way your contributor agreement is written, it seems to assume that the individual author of the code, the GitHub account holder, here @asoldano, has the authority to sign this type of agreement, which is a copyright assignment, and can make representations to that effect. In the case of this particular pull request, it is likely that Red Hat owns the copyright. Individual Red Hat-employed developers are generally not authorized to sign copyright assignments covering Red Hat-copyrighted code. (The original agreement this was based on, the Sun/Oracle contributor agreement, does not have this problem - it assumes corporate signature as a normal mode of signing.)

Do you have an alternative version of this agreement that can be signed by someone with corporate authority rather than a GitHub account holder? Or preferably, some licensing arrangement rather than copyright assignment, which is a somewhat odd requirement especially for an Apache License 2.0 project. For example, we'd be happy to license this code directly to you under the Apache License itself.

(Gratuitously, I would point out that joint copyright assignment is not a mainstream approach to handling contributions to open source projects. Today it's mainly associated with some legacy Sun Microsystems projects maintained by Oracle. I think Oracle is continuing with this approach purely for reasons of inertia. It's hard to see what you are gaining from joint copyright assignment (given its legal drawbacks from your perspective) that you couldn't achieve from a more mainstream approach like the Apache CLA model, though I'm a critic of that as well.)

@cowtowncoder
Copy link
Member

@richardfontana Yes, we have the alternative corporate-CLA too:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement-corporate.txt

which I think would then work better here. We have one for 3 other major corporations who all prefer it over individual one; it is a much more recent addition. Original individual CLA came from Sun, as what seemed to be the mainline back in the day (2008 or so). And we are also using it due to inertia.

Sometimes I wonder if just writing all of my own code might be simpler than accepting code contributions at all; but so far CLA/CCLA have worked well enough from simplistic workflow perspective.

@alexander-riss
Copy link

when will this change be available in a released version? this seems quite important for server side use cases with user provided data

@cowtowncoder
Copy link
Member

@alexander-riss I think this could be added in a patch release. Given our current open branches, that would be 2.8.4 (to be released sooner, within a week or two) and 2.7.9 (will take longer as rate of fixes much lower).

@cowtowncoder
Copy link
Member

@alexander-riss @richardfontana does the CCLA work for you? I would be happy to merge this feature in, and agree that it is a valuable addition.

@richardfontana
Copy link

@cowtowncoder This CCLA is acceptable for us. It may take a while to get it signed by the appropriate executive.

@cowtowncoder
Copy link
Member

@richardfontana Understood, thank you for getting this done.

@nilstgmd
Copy link

@richardfontana @cowtowncoder What is the status on that issue? This fix is quite relevant for us. Thanks.

@cowtowncoder
Copy link
Member

@nilstgmd As per messages above, waiting for Contributor License Agreement.

@cowtowncoder
Copy link
Member

@richardfontana I hope you are making progress wrt CCLA. If this could be resolved relatively soon, like within a week, I think patch could go in 2.8.6, to be released relatively soon. If not, it'll go in a following patch.

@richardfontana
Copy link

@cowtowncoder not sure if this is too late for 2.8.6 but CLA has now been signed and emailed to you.

@cowtowncoder
Copy link
Member

@richardfontana Excellent -- not too late for 2.8.6!

@cowtowncoder cowtowncoder merged commit 4616602 into FasterXML:2.8 Jan 12, 2017
@cowtowncoder
Copy link
Member

@asoldano Thank you for contributing this -- it will be included in 2.8.6.

@cowtowncoder cowtowncoder modified the milestones: 2.8.0, 2.8.6 Jan 12, 2017
@asoldano
Copy link
Contributor Author

:-)

ccaominh added a commit to ccaominh/druid that referenced this pull request Nov 26, 2019
Addresses security vulnerabilities:

- sonatype-2016-0397:
  FasterXML/jackson-core#315

- sonatype-2017-0355:
  FasterXML/jackson-core#322
fjy pushed a commit to apache/druid that referenced this pull request Nov 27, 2019
Addresses security vulnerabilities:

- sonatype-2016-0397:
  FasterXML/jackson-core#315

- sonatype-2017-0355:
  FasterXML/jackson-core#322
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

Successfully merging this pull request may close these issues.

5 participants