FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.52k stars 1.38k forks source link

Add check in primitive value deserializers to avoid deep wrapper array nesting wrt `UNWRAP_SINGLE_VALUE_ARRAYS` [CVE-2022-42003] #3590

Closed cowtowncoder closed 2 years ago

cowtowncoder commented 2 years ago

TL;DNR:

Fix included in:


(note: similar to #3582 ) (note: originally found via oss-fuzz https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51020)

Implementation of methods like _parseBooleanPrimitive (in StdDeserializer) uses idiom:

            if (ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS)) {
                p.nextToken();
                final boolean parsed = _parseBooleanPrimitive(p, ctxt);
                _verifyEndArrayForSingle(p, ctxt);
                return parsed;
            }

to handle unwrapping. While simple this exposes possibility of "too deep" nesting and possible problem with resource exhaustion in some cases. We should change this similar to how #3582 was handled.

cowtowncoder commented 2 years ago

Methods to update (and regression test):

Also note that method _deserializeWrappedValue() implements checks although cannot quite be called as-is. Similarly, BeanDeserializer._deserializeFromArray() at around line 632 has similar checks.

henryrneh commented 2 years ago

Hello dear @cowtowncoder,

I am Henry from Code Intelligence. First of all thank you for your quick fixes of this issue!

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51020

Is this issue regarded as a security issue? If yes, we are thinking about applying CVE for it, so the community knows about it and will update to the latest version of jackson-databind.

Thank you for your fixes and support for OSS-Fuzz!

Best regards, Henry

cowtowncoder commented 2 years ago

@henryrneh Same as #3582 (see my comments there).

henryrneh commented 2 years ago

@cowtowncoder thank you :+1:

DavidKorczynski commented 2 years ago

This issue was found by a fuzzer written by the Ada Logics team and is part of an ongoing security assessment. @henryrneh can you please ensure the issues you report are found by the fuzzers written by your team (https://github.com/google/oss-fuzz/blob/master/projects/jackson-core/JsonFuzzer.java and https://github.com/google/oss-fuzz/blob/master/projects/jackson-databind/ObjectReaderFuzzer.java) then we'll take care of those from our fuzzers.

henryrneh commented 2 years ago

I already canceled the application. We will do our best to try not to apply CVEs for fuzz targets written by AdaLogics, however we will need some assitance or notification by you to know who wrote which fuzz target, because OSS-Fuzz is not designed to support this, maybe you can use some special prefix in the fuzz target name so it's more obvious for us so we can filter it out?

pjfanning commented 2 years ago

@henryrneh the disclosure in https://nvd.nist.gov/vuln/detail/CVE-2022-42003 should really have been delayed until jackson-databind v2.14.0 was released. The RC should have been given a good chance to be tested and an orderly release of 2.14.0 done when it is ready. Now we risk having people clamouring for an early release of 2.14.0.

DavidKorczynski commented 2 years ago

@pjfanning -- @AdamKorcz and I applied for this CVE and was done in coordination with the current audit of jackson. We coordinated this with Tatu in terms of agreeing we should apply for CVEs for these issues.

jgoeres commented 2 years ago

Our OWASP scan just ran into this finding for our use of jackson-databind 2.13.3. We are very hesitant to go to an RC of 2.14 (and of course fully understand that this issue cannot change the release plans for 2.14, so we will not join the crowd that is pushing for an "early release of 2.14" @pjfanning :-)). But since 2.13 is an actively maintained branch according to https://github.com/FasterXML/jackson#active-developed-jackson-2x-branches, I assume a fix will be made available there as well? Any activities or maybe even an ETA for that?

pjfanning commented 2 years ago

@jgoeres @alzimmermsft are you even using the UNWRAP_SINGLE_VALUE_ARRAYS option?

alzimmermsft commented 2 years ago

I'll second what @jgoeres, that we're getting security alerts around usage of Jackson 2.13.4 and won't be able to upgrade to the 2.14 RC any time soon and downstream consumers of our SDKs will also get alerts as well. We aren't using UNWRAP_SINGLE_VALUE_ARRAYS but a lot of noise will still be caused.

Edit:

I know that the fix for this issue now results in an exception being thrown, which is a runtime breaking change within a patch release, but there has been prior art on this being accepted with the fix for https://github.com/FasterXML/jackson-databind/issues/2816. There was also API changes as well, though looking at the fix for this issue the shared method could be made non-protected if it's backported and shouldn't be exposed.

pjfanning commented 2 years ago

@pjfanning -- @AdamKorcz and I applied for this CVE and was done in coordination with the current audit of jackson. We coordinated this with Tatu in terms of agreeing we should apply for CVEs for these issues.

Just watch the comments flow in looking for special attention now this change is made public. I've already had https://github.com/swagger-akka-http/swagger-akka-http/issues/342

jgoeres commented 2 years ago

@pjfanning We have roughly dozen microservices spread across as many teams, we haven't checked for all of them. But even if none of them used that setting, scans done in various places (both within our organization, but also by customers that run this on-prem) will find the lib and this will lead to irritation and demands to resolve it by updating even if we claim that it is not exploitable.

cowtowncoder commented 2 years ago

@DavidKorczynski I think we need coordinate better in future: my understanding was that a CVE would be filed, I'd be notified with id allocated (so I can update this issue, release notes), but CVE only made public once official release was out. But perhaps I am assuming there is a mechanism that does not exist between allocating an id and making it available for the public.

What is needed at minimum, at any rate, is the linkage between CVE and issues. The problem right now is that users email me or file issues asking "what's up with CVE-2022-xxxx" and then I have to go digging. It's not a huge effort of course but is an interruption, and something we can avoid by collaboration.

We may also need to discuss some of finer details: in this case I hope I mentioned (but perhaps I didn't?) that this setting is disabled by default -- that should have an effect in ranking.

I realize I did not quite realize everything that needs to be discussed before I say it's fine to proceed with CVE filing. Live and learn. Thank you everyone.

cowtowncoder commented 2 years ago

Note: also added the "sibling" CVE marker (2022-42004) for #3582.

sbernard31 commented 2 years ago

@cowtowncoder do you know approximately when 2.14.0 will be released ? We plan to release a new version of our library soon and I would like to know that to decide if we should wait for 2.14.0 or go with 2.14.0-rc1.

rzo1 commented 2 years ago

I agree with @sbernard31 - any ETA for 2.14.0 final would be great as we are also looking ahead to go for a new release vote soon.

pjfanning commented 2 years ago

See https://groups.google.com/g/jackson-dev/c/RuiMDNM3vpQ/m/FgLnTxBPAwAJ and follow that thread for updates

sbernard31 commented 2 years ago

I understand the final 2.14.0 release is for "maybe around mid-October in the best case scenario." So we will go for 2.14.0-rc1 fo now.

@pjfanning thx for the answer :pray:

cowtowncoder commented 2 years ago

2.14.0 will be out when it's ready. We are too small a project to have much of an estimation process. But I think that:

  1. I want to do 2.14.0-rc2 due to number of recent fixes, changes
  2. Java One is on October 17 - 20, and I'll attend, probably won't get 2.14.0 before
  3. So 2.14.0 out, I hope, by end of October 2022.

As to 2.14.0-rc1 -- as usual, please do note that it is not a Production release. Use at your own risk. That said, no critical problems have been found so far.

cowtowncoder commented 2 years ago

@sbernard31 Please, whatever you do, please DO NOT RELEASE A PUBLIC VERSION with 2.14.0-rc1 dependency. That would add non-production dependencies throughout ecosystem.

If releasing RC, pre-release, alpha, beta etc, those are fine; most dependency tracking systems recognize these as non-production releases.

sbernard31 commented 2 years ago

Thx @cowtowncoder for prompt answer. It's for a milestone release so I guess it's OK ?

please DO NOT RELEASE A PUBLIC VERSION with 2.14.0-rc1 dependency. That would add non-production dependencies throughout ecosystem.

I will not but I will not be surprised if some Jackson consumers do that reading :

In FasterXML jackson-databind before 2.14.0-rc1, resource exhaustion can occur because 
of a lack of a check in primitive value deserializers to avoid deep wrapper array nesting,
when the UNWRAP_SINGLE_VALUE_ARRAYS feature is enabled.

As there is no other available release fixing those issue. :thinking:

I want to do 2.14.0-rc2 due to number of recent fixes, changes

Do you plan to do it today ?

cowtowncoder commented 2 years ago

No, I am not planning to release 2.14.0-rc2 today. I hope to do it before Java One -- this weekend would be the earliest; if not, next week.

On 2.14.0-rc1: I am fine with users using it if they feel compelled to do so due to "security" tools (I wish CVE was not released yet but...). That is their choice. I just hope it will not be automated via dependency resolution.

chadlwilson commented 2 years ago

@DavidKorczynski I think we need coordinate better in future: my understanding was that a CVE would be filed, I'd be notified with id allocated (so I can update this issue, release notes), but CVE only made public once official release was out. But perhaps I am assuming there is a mechanism that does not exist between allocating an id and making it available for the public.

@cowtowncoder the mechanism definitely exists with MITRE to reserve and allocate a CVE independently of disclosure, which is normal practice. Indeed if one uses GHSA to raise ones security isues you can get GitHub to manage this process for you as a CVE Numbering Authority, as well as collaborate on the text in GitHub with those who raise the issue, leaving maintainers able to manage the disclosure alongside, or after a release is available.

Based on what I observed with the set of ossfuzz SnakeYAML vulns in the last few months and now this, the well intentioned and otherwise good work within ossfuzz is in some cases unleashing a torrent of noise on already limited OSS dev time due to the way our current generation of tools work (dependency checkers etc), complexity of dependency trees and then overreaction from certain ops/security folk who ignore important context and for some reason put blind trust in the CVE process (which is remarkably, err, subjective and subject to abuse IMHO).

Unfortunately this case does not seem like 'responsible' disclosure to me. I fear that if we as a community create noise like this, we take away from dev time (OSS and users alike) to actually address the most important security issues - instead replacing it with devs having to understand or explain risk severity, evaluate environmental context/feature usage yada yada, rather than just being able to upgrade and move on (ideally bot-and-test-automation assisted) - without having to think about whether one is affected or not.

cowtowncoder commented 2 years ago

Very well put @chadlwilson.

jgoeres commented 2 years ago

I'll second what @jgoeres, that we're getting security alerts around usage of Jackson 2.13.4 and won't be able to upgrade to the 2.14 RC any time soon and downstream consumers of our SDKs will also get alerts as well. We aren't using UNWRAP_SINGLE_VALUE_ARRAYS but a lot of noise will still be caused.

Edit:

I know that the fix for this issue now results in an exception being thrown, which is a runtime breaking change within a patch release, but there has been prior art on this being accepted with the fix for #2816. There was also API changes as well, though looking at the fix for this issue the shared method could be made non-protected if it's backported and shouldn't be exposed.

Since everybody seems to be only talking about fixing this for 2.14 - is the verdict out yet if there will be a fix for that on 2.13.x, for the IMNSHO well-founded reasons explained by @alzimmermsft and myself?

pjfanning commented 2 years ago

the discussion about whether a v2.13 patch is made is on https://github.com/FasterXML/jackson/discussions/126

cowtowncoder commented 2 years ago

As per @pjfanning provided link, my thinking is that it is somewhat risky to backport and hence 2.14.0 only.

It is also not necessarily the case that I'd have any time to release 2.13.5 sooner than 2.14.0: my time to use on Jackson is limited and I need to prioritize it heavily.

plaird commented 2 years ago

@chadlwilson

unleashing a torrent of noise on already limited OSS dev time....and then overreaction from certain ops/security folk who ignore important context and for some reason put blind trust in the CVE process (which is remarkably, err, subjective and subject to abuse IMHO).

Totally agree, and it goes beyond this unfortunately. Those of us who operate within regulatory frameworks (e.g. gov) have contractural obligations to remediate within specific time frames. We aren't just debating with overzealous sysadmins, we are beholden to external auditors who aren't very interested in the mitigating context. A vuln is a vuln to them.

cowtowncoder commented 2 years ago

Yes @plaird this is very true. At my day jobs I have had a chance to observe this as well. It's a regular mess when all of these things collide... assumptions made on validity of specific vulnerability reports, inflexibility of modeling and so on.

cowtowncoder commented 2 years ago

Ok, on backporting. So, I am really time-bound and focused on getting 2.14 out -- 2.14.0-rc2 was released today.

But I would consider micro-patch for jackson-databind, if anyone has time to do a PR against 2.13 (just cherry-picking changes needed here). If so, I could release 2.13.4.1 for databind and matching jackson-bom as this does not take very long. Unlike spending half a day to push out 2.13.5 with no real changes except for this one thing.

rzo1 commented 2 years ago

if anyone has time to do a PR against 2.13 (just cherry-picking changes needed here).

@cowtowncoder Here you go: https://github.com/FasterXML/jackson-databind/pull/3621

cowtowncoder commented 2 years ago

Ok, as per edit on description:

Fix included in

chadlwilson commented 2 years ago

Thanks a lot @cowtowncoder . I have contacted NVD/NIST with the relevant information and am seeking to get https://nvd.nist.gov/vuln/detail/CVE-2022-42003 updated to reflect these micro-patch releases so people's scanner noise goes away if they update to these (or via relevant BOMs).

Will follow up for the separate CVE fix backported to 2.12.x if I get a positive response/outcome.

cowtowncoder commented 2 years ago

Thank you @chadlwilson much appreciated.

cowtowncoder commented 2 years ago

NOTE: unfortunately 2.13.4.1 of jackson-databind has an issue that affects Gradle users -- so I had to publish 2.13.4.2 (with bom 2.13.4.20221013). Please use latter instead.

vdotjansen commented 2 years ago

@chadlwilson

Thanks a lot @cowtowncoder . I have contacted NVD/NIST with the relevant information and am seeking to get https://nvd.nist.gov/vuln/detail/CVE-2022-42003 updated to reflect these micro-patch releases so people's scanner noise goes away if they update to these (or via relevant BOMs).

Will follow up for the separate CVE fix backported to 2.12.x if I get a positive response/outcome.

They updated the CVE as the description changed from "In FasterXML jackson-databind before 2.14.0-rc1, resource exhaustion can occur because of a lack of a check in primitive value deserializers to avoid deep wrapper array nesting, when the UNWRAP_SINGLE_VALUE_ARRAYS feature is enabled." to "In FasterXML jackson-databind before 2.14.0-rc1, resource exhaustion can occur because of a lack of a check in primitive value deserializers to avoid deep wrapper array nesting, when the UNWRAP_SINGLE_VALUE_ARRAYS feature is enabled. Additional fix version in 2.13.4.1 and 2.12.17.1".

But they forgot to update the CPE, therefor it will still match the new versions.

I e-mailed them because I mostly look at the CPE, did not notice the change in the description at first.

radarsh commented 2 years ago

did not notice the change in the description at first.

You are not alone.

Have they been informed about 2.13.4.2 as well?

chadlwilson commented 2 years ago

They updated the CVE as the description changed from "In FasterXML jackson-databind before 2.14.0-rc1, resource exhaustion can occur because of a lack of a check in primitive value deserializers to avoid deep wrapper array nesting, when the UNWRAP_SINGLE_VALUE_ARRAYS feature is enabled." to "In FasterXML jackson-databind before 2.14.0-rc1, resource exhaustion can occur because of a lack of a check in primitive value deserializers to avoid deep wrapper array nesting, when the UNWRAP_SINGLE_VALUE_ARRAYS feature is enabled. Additional fix version in 2.13.4.1 and 2.12.17.1".

But they forgot to update the CPE, therefor it will still match the new versions.

@vdotjansen What you possibly need to know (but may not?) is that descriptions as they appear in NVD come from and are mastered in MITRE rather than the NVD -- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-42003 . Likely someone has updated it there; possibly via the CNA/submitter or with a form submission to MITRE itself and the description has flowed through to the NVD in feeds. That won't do anything for CVSS scores, CPEs, affected versions etc. These parts are mastered in the NVD and MITRE knows nothing about them. When changes flow through from MITRE, they tend to get marked as

This vulnerability has been modified since it was last analyzed by the NVD. It is awaiting reanalysis which may result in further changes to the information provided.

So they didnt "forget" to update the CPE, they just haven't updated it yet.

I e-mailed them because I mostly look at the CPE, did not notice the change in the description at first.

NVD haven't actually replied to my original email, and it's still marked as being under analysis due to this description change - so they will probably get to it. I'd advise being patient. I've included the details already about the CPEs needing adding as well as the adjustment to the versions. I've done this a few times before, and it has always worked ... eventually.

Have they been informed about 2.13.4.2 as well?

@radarsh It's not strictly necessary for them to have every version listed unless it's at the boundary of a fix or vuln, but since it is still in their analysis queue, they will likely add that version at the same time.

The process is slow and manual and non-transparent, but I suspect 100 of us emailing them isn't going to make it go any faster....

vdotjansen commented 2 years ago

@vdotjansen What you possibly need to know (but may not?) is that descriptions as they appear in NVD come from and are mastered in MITRE rather than the NVD -- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-42003 . Likely someone has updated it there; possibly via the CNA/submitter or with a form submission to MITRE itself and the description has flowed through to the NVD in feeds. That won't do anything for CVSS scores, CPEs, affected versions etc. These parts are mastered in the NVD and MITRE knows nothing about them. When changes flow through from MITRE, they tend to get marked as

I knew that there was data shared between MITRE and NVD, but did not know which part came from MITRE although I did know at least the CPE's are managed by NVD.

NVD haven't actually replied to my original email, and it's still marked as being under analysis due to this description change - so they will probably get to it. I'd advise being patient. I've included the details already about the CPEs needing adding as well as the adjustment to the versions. I've done this a few times before, and it has always worked ... eventually.

Like I said I only checked the CPE.

The process is slow and manual and non-transparent, but I suspect 100 of us emailing them isn't going to make it go any faster....

You are correct and I should have checked here first. My e-mail was send before I noticed your message here. We should not all send an e-mail you are correct. I should have checked here to see, but I think it is better to send mails by 2 different people then 0 mails, as they need to know this. Next time I will check the issues to see if someone already send a mail. Me posting here that I also send an e-mail is hoping there will not be more e-mails towards them about this :)

chadlwilson commented 2 years ago

NIST/NVD have got back to me after reviewing the publicly available info and are updating the affected versions right now.

I am working with the assumption that all 2.11.x and earlier versions are also affected, but if the UNWRAP_SINGLE_VALUE_ARRAYS feature in question was only added some time along the way in a 2.x version the CVE mapping could be more specific. Possibly a bit moot given the other vulnerabilities likely to exist in those earlier versions anyway...

chadlwilson commented 2 years ago

The CVE affected versions for this issue are finally updated and should be correct now. There was a bit of back-and-forth with the NIST/NVD folks and a weekend delay, but got there eventually...

RetroDreams commented 2 years ago

@chadlwilson Maybe I just need to wait for another database update but 26 hours ago my Anchore vuln scanner reported this was fixed via 2.13.4.1 and now my scanner is reporting that NVD says CVE-2022-42003 has no fix available. This is directly contradictory to what is displayed on NIST? I am thoroughly confused.

{
            "feed": "vulnerabilities",
            "feed_group": "nvd",
            "fix": "None",
            "nvd_data": [
                {
                    "cvss_v2": {
                        "base_score": -1.0,
                        "exploitability_score": -1.0,
                        "impact_score": -1.0
                    },
                    "cvss_v3": {
                        "base_score": 7.5,
                        "exploitability_score": 3.9,
                        "impact_score": 3.6
                    },
                    "id": "CVE-2022-42003"
                }
            ],
            "package": "jackson-databind-2.13.4.1",
            "package_cpe": "None",
            "package_cpe23": "cpe:2.3:a:fasterxml:jackson-databind:2.13.4.1:*:*:*:*:*:*:*",
            "package_name": "jackson-databind",
            "package_path": "/root/.gradle/wrapper/dists/gradle-7.5.1-bin/7jzzequgds1hbszbhq3npc5ng/gradle-7.5.1/lib/plugins/jackson-databind-2.13.4.1.jar",
            "package_type": "java",
            "package_version": "2.13.4.1",
            "severity": "High",
            "url": "https://nvd.nist.gov/vuln/detail/CVE-2022-42003",
            "vendor_data": [],
            "vuln": "CVE-2022-42003",
            "will_not_fix": false
}
chadlwilson commented 2 years ago

@RetroDreams not sure. Seems like something you'd need to check in your Anchore deployment.