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

Issue sonatype-2022-6438 (fixed via #827) #861

Closed
chrisgleissner opened this issue Dec 8, 2022 · 30 comments
Closed

Issue sonatype-2022-6438 (fixed via #827) #861

chrisgleissner opened this issue Dec 8, 2022 · 30 comments
Milestone

Comments

@chrisgleissner
Copy link

chrisgleissner commented Dec 8, 2022

Hi,

We are using jackson-core:2.13.4. A few hours ago, we started getting alerts that this version had the following security vulnerability and that even the latest available version 2.14.1 was affected. Thus, no known remediation path exists.

Are you aware of the issue and when do you plan to release a fix? Also, is there anything we can do in the mean-time to mitigate the issue?

Thanks
Christian

Issue sonatype-2022-6438:

"The jackson-core package is vulnerable to a Denial of Service (DoS) attack. The methods in the classes listed below fail to restrict input size when performing numeric type conversions. A remote attacker can exploit this vulnerability by causing the application to deserialize data containing certain numeric types with large values. Deserializing many of the aforementioned objects may cause the application to exhaust all available resources, resulting in a DoS condition.

Vulnerable File(s) and Function(s):

com/fasterxml/jackson/core/base/ParserBase.class

_parseSlowInt()
convertNumberToBigDecimal()
com/fasterxml/jackson/core/base/ParserMinimalBase.class

getValueAsDouble()
com/fasterxml/jackson/core/util/TextBuffer.class

contentsAsDecimal()
contentsAsDouble()
contentsAsFloat()
Detection
The application is vulnerable by using this component if it does not restrict user-supplied numeric input values prior to deserialization.

Recommendation
There is no non-vulnerable upgrade path for this component/package. We recommend investigating alternative components or a potential mitigating control.

Version Affected
[2.0.0-RC1,2.14.1]"

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 8, 2022

Very nice of whoever reported this to not bother contacting the project first. I have not seen a PoC of potential attack either so as far as I know this is unproven so seems wrong to have been published.

There is work in 2.15 to resolve these concerns but not for 2.14 or earlier; no backporting possible as configuration and plumbing requires significant changes, testing etc.

Fuck I hate this security theater, rogue researchers.

/cc @pjfanning

@cowtowncoder
Copy link
Member

Also, belongs to jackson-core as that's where relevant handling would be, will transfer.

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-databind Dec 8, 2022
@cowtowncoder
Copy link
Member

@chrisgleissner Aside from everything else, it is nonsense to include lower-level methods without considering actual code path: strongly-typed data (POJOs) would not necessarily refer to types (BigDecimal, possibly BigInteger) that allow targeting; and even if they do attacker would need to tailor it to known target.

I suppose CVE would refer to use with "untyped" values (readValue(Object.class)) or possibly tree mode (readTree(...)) but without more details it's hard to say.

And once again this is a good example of NON-responsible disclosure. Shame on whoever published it.

@pjfanning
Copy link
Member

Someone probably saw the recent commits and decided to glory hunt. Shame on mitre for accepting this.

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 9, 2022

Yes, was thinking the same.

I think this might move up timeline for 2.15. Not by huge amount (there's holiday season now etc) but may result in enough pressure for "mini-minor version".

I really need to get started on investigating property introspection rewrite. Even if incomplete, but get something started for 2.15.

@pjfanning
Copy link
Member

Anyone who gets this security warning should check if their application handles inputs from untrusted sources. If not, then you are safe to ignore this.
If you do allow untrusted inputs in your application, then there are many ways for malicious users to cause excessive resource use in your app. You should be looking at applying network traffic rules to check for issues before trying to parse the inputs. Your application should be built to recover from failures.

@chrisgleissner
Copy link
Author

chrisgleissner commented Dec 9, 2022

Thanks @cowtowncoder and @pjfanning , that is good reasoning. For Intranet applications with authenticated users, such DoS-type security issues are much less relevant. It indeed appears as though a public pull request mentioning possible security impacts (#827, slow processing times) is sufficient to create a CVE. Thanks a lot for working on this, can't wait for 2.15 to come out.

@OrangeDog
Copy link

@pjfanning Mitre haven't accepted this. It doesn't have a CVE.

@chrisgleissner
Copy link
Author

chrisgleissner commented Dec 9, 2022

@OrangeDog As you correctly point out (thanks), it is not a Mitre CVE but rather a Sonatype CVSS issue, see https://help.sonatype.com/iqserver/product-information/sonatype-vulnerability-data for their rating system. I changed the title accordingly and replaced 'CVE' with 'Issue'.

@chrisgleissner chrisgleissner changed the title CVE sonatype-2022-6438 for jackson 2.0.0-RC1 through 2.14.1 without an upgrade path Issue sonatype-2022-6438 for jackson 2.0.0-RC1 through 2.14.1 without an upgrade path Dec 9, 2022
@cowtowncoder
Copy link
Member

Based on some off-ticket discussion it sounds like this report sort of escaped to public sphere.
But it is also based on an older advisory so I wonder whether it might be addressed to some degree by work @pjfanning added in 2.14 already (wrt more efficient handling of BigDecimal and BigInteger -- including lazy construction which may avoid some problems too).

@deepblueli
Copy link

Based on some off-ticket discussion it sounds like this report sort of escaped to public sphere. But it is also based on an older advisory so I wonder whether it might be addressed to some degree by work @pjfanning added in 2.14 already (wrt more efficient handling of BigDecimal and BigInteger -- including lazy construction which may avoid some problems too).

It would great if that's possible as this is picked up as part of the security scanner in my company. Would save us a lot of paperworks if this can be done in 2.14 without waiting for 2.15.

@cowtowncoder
Copy link
Member

Yes. I am not quite sure how to go about this tho.

@jensborrmann
Copy link

Not really daring to ask... Do you consider backports to older branches (2.12, 2.13)?

@brianf
Copy link

brianf commented Dec 15, 2022

I just wanted to come in here and follow up a bit from the Sonatype perspective.

First and most importantly, when we believe we have found a zero-day, we ABSOLUTELY follow a zero day process and find contact with the project privately and move to help acquiring a CVE and then coordinate disclosure.

Separate from that, we have software that looks for relevant indicators of vulnerability fixes across many things including GitHub events. This was developed as a necessity (@cowtowncoder you might have even helped build this back in the day) because we found a long time ago that developers might fix an issue but never record a CVE. As at the time, this data was for customers only, we would assign a trackable number (Sonatype-xxx) and in our commercial tools, it is easy for customers to assess the implications and then manage it accordingly. Since often times the project had their own reasons for not filing a CVE, we do not go to a dispute process by habit and instead just erred on the side of providing more of an -advisory- finding for our products.

Fast forward a few years, we recently consolidated the databases behind OssIndex and our commercial products. This meant that lots of proprietary data is now available via the OSSIndex and the tools that consume it such as DependencyCheck, DependencyTrack and many others.

We are working to figure out how to best approach this new reality. We want to provide advisories when we feel it's relevant -- even if a project disputes that something is a "vulnerability", doesn't mean that is not a useful to know finding. In other words, Many cases here apply to components and the component developers might legitimately say that's a user/developer responsibility...but the consumers of our tools and feeds are in fact those developers/users and they should know to eg whitelist classes for deserialization or validate certain parameters.

To specifically address how this all fits together into this conversation, It looks like our tools picked up on the conversation about a potential DoS vulnerability in a recent pull request. This went to our team of researchers who confirmed this was a legitimate hit and flagged it accordingly as something to alert our customers about. We looked closer at this case and have adjusted our process such that if the hit appears to be an inflight conversation that we make it an escalation process so others such as myself can work with the project to ensure we aren't inadvertently shining a light where it shouldn't be.

It is important to note that because all of these findings we are picking up in this process are in fact already public we did not consider that a zero-day process was applicable. In other words, if our systems picked up these types of things, you can guarantee the bad guys did as well and in that light, nothing new was being disclosed by our system, we simply indexed it and pointed users at it.

We will continue to work to make some of this more clear in the Sonatype id findings via the api. We don't control the usage of the api in all the tools and so the best experience will always be in the tools we do control and provide, both free and paid.

@cowtowncoder
Copy link
Member

Not really daring to ask... Do you consider backports to older branches (2.12, 2.13)?

No. Nothing related to added limits can really be backported as far as I can see.

@albertwangnz
Copy link

Based on some off-ticket discussion it sounds like this report sort of escaped to public sphere. But it is also based on an older advisory so I wonder whether it might be addressed to some degree by work @pjfanning added in 2.14 already (wrt more efficient handling of BigDecimal and BigInteger -- including lazy construction which may avoid some problems too).

This sounds great. If @brianf , on behalf of SonarType, can accept and update their database, it would save lots of paperwork for many developers~

@abdullahali99
Copy link

Any update on this issue?

@albertwangnz
Copy link

Hi @cowtowncoder ,

Happy new year!

I just want to check if there is any update. Do you think the fix from @pjfanning already solved the issue? Or would some solutions be targeted to 2.15?

Thanks.

Ablert

@cowtowncoder
Copy link
Member

@albertwangnz PR #827 was merged in 2.15 over a month ago and targets the issue reported.

In fact I assume that PR triggered creation of the issue (which is a really perverse thing -- if we did NOT fix something, there'd be no issue. Go figure).

@albertwangnz
Copy link

Thanks, @cowtowncoder . Do you know the plan of the 2.15 release? Cheers.

@cowtowncoder
Copy link
Member

@albertwangnz No, no current plans. Minor versions are typically released once or twice a year and 2.14 was only released in November. So without this hilarious Security Theater bullshit we would probably not be releasing it until mid-2023.

But with pressure that gets added we will probably have to spend more time (overhead) in releasing 2.15.0 earlier. I still do not want to do it with just what exists now:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15

so... maybe in March? I don't know, "when it's ready" is the usual: project has never been date/deadline driven as that rarely makes sense for OSS projects (except ones with corporate backing, with commercial reasons for dates).

@brianf
Copy link

brianf commented Jan 9, 2023

I can't help but think that the pressure here is coming from an expectation of "vulnerability zero" by users of tools that can't nuance what the data is telling them. I've been thinking about blogging this for a bit, but Josh Bressers did so recently[1] and it covers the challenge here. The point being, the item in the ossindex has made users aware of a potential issue. They should be able to assess their exposure and deal with it accordingly. The issue at hand seems remote and so constantly asking the team to make an update just to take it off your list is an anti pattern. [1] https://opensourcesecurity.io/2023/01/03/the-perverse-incentive-of-vulnerability-counting/

@albertwangnz
Copy link

@albertwangnz No, no current plans. Minor versions are typically released once or twice a year and 2.14 was only released in November. So without this hilarious Security Theater bullshit we would probably not be releasing it until mid-2023.

But with pressure that gets added we will probably have to spend more time (overhead) in releasing 2.15.0 earlier. I still do not want to do it with just what exists now:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15

so... maybe in March? I don't know, "when it's ready" is the usual: project has never been date/deadline driven as that rarely makes sense for OSS projects (except ones with corporate backing, with commercial reasons for dates).

Hi @cowtowncoder thank you for your reply. I fully understand your point. No pressure. I just wanted to know if there is a plan. If not I would go to add an exemption to our project. Since I need to complete some paperwork, so I prefer not to if a release would solve the problem soon LOL.

I appreciate all your effort on the OSS project which could help so many developers.

Albert

@albertwangnz
Copy link

albertwangnz commented Jan 9, 2023

Hi @brianf thank you for sharing the blog. It is instructive. I will share the blog with my bosses first.

I think our (and maybe some other developers') problem is how can we convince our customers to understand lol. Especially when we provide "Software as a Product", and some VIP customers may even scan the deployed products by themselves.

I guess the the result will become those developers need to assess each potential issue, understand the usage of the library and understand how their products use the library :-) If we consider how many developers we have and how many people are exposing more and more potential issues, it becomes a hard and very time-consuming task.

But whatever I totally agree with you that nobody should PUSH the OSS team to make updates to solve those potential issues. Their contribution already helps so many developers and I really respect them.

@OrangeDog
Copy link

developers need to assess each potential issue, understand the usage of the library and understand how their products use the library

That is the job description of a software developer, yes.

@plaird
Copy link

plaird commented Jan 10, 2023

@brianf some comments...

I can't help but think that the pressure here is coming from an expectation of "vulnerability zero" by users of tools that can't nuance what the data is telling them.

Maybe in some cases. My pressure is coming from regulatory requirements. Regulatory frameworks don't care about nuance. Every open vuln carries a binding requirement on us to litigate with non-technical auditors. This is the curse that open disclosure inflicts on technical teams.

We are working to figure out how to best approach this new reality.

Please consider impacts on regulatory frameworks in this new reality. When your researchers write "The jackson-core package is vulnerable to a Denial of Service (DoS) attack" the auditors take that as fact and it is up to us to prove how we have mitigated it. If there is no fix available from the project team, that places a big burden on users.

We looked closer at this case and have adjusted our process such that if the hit appears to be an inflight conversation that we make it an escalation process so others such as myself can work with the project to ensure we aren't inadvertently shining a light where it shouldn't be.

👍 Yes, this is needed. The escalation process should consider: "what is the impact of disclosure on regulated industries".

@cowtowncoder
Copy link
Member

Not much I can add: good discussion all around.

From my part maybe I should start planning for 2.15.0 as the situation "is what it is". I don't think there is any way around the fact that the disclosure has started a chain of events that has increased the need to get a patched version out. And in many ways it is not a bad thing if there was a yet "smaller" Jackson minor release this year.
Maybe not what I was planning for, but perhaps what was needed anyway. :)

@rabbit20210221
Copy link

rabbit20210221 commented Mar 29, 2023

Hi guys,
I just checked Jackson Release 2.15 wiki and this ticket isn't listed on the changelog https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15
Just wondering if this ticket is still being considered to be addressed as part of Jackson 2.15 release?

Edit: i just saw mention of #827 that implements the fix for this issue and has been merged.

@cowtowncoder cowtowncoder changed the title Issue sonatype-2022-6438 for jackson 2.0.0-RC1 through 2.14.1 without an upgrade path Issue sonatype-2022-6438 (fixed via #837) Mar 31, 2023
@cowtowncoder cowtowncoder added this to the 2.15.0 milestone Mar 31, 2023
@cowtowncoder cowtowncoder changed the title Issue sonatype-2022-6438 (fixed via #837) Issue sonatype-2022-6438 (fixed via #827) Mar 31, 2023
@cowtowncoder
Copy link
Member

@rabbit20210221 Yes, in this case reference to PR was included (not sure why/how, probably unintentionally). I'll update title here, close; fix is included in 2.15.

@levinebw
Copy link

I absolutely agree with @brianf

I can't help but think that the pressure here is coming from an expectation of "vulnerability zero" by users of tools that can't nuance what the data is telling them. They [the consumer] should be able to assess their exposure and deal with it accordingly. The issue at hand seems remote and so constantly asking the team to make an update just to take it off your list is an anti pattern.

The unfortunate reality is that the majority of consumers are content to rely on the output of the scan report as the sole source of truth on the severity of an issue; without giving any treatment to whether it impacts their system or not. Where users do have a procedure to 'adjudicate', 'suppress' the finding due to evidence that their system is not affected or other mitigations, the process often entails so much enterprise procedural friction that removing the issue (by forcing someone to patch something) is easier than going through the effort to analyze the finding and issue an exception.

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