Got security warning on JDom2 2.0.6: CVE-2021-33813

[steinarb]

I got a security warning on JDom 2.0.6, because of CVE-2021-33813: https://nvd.nist.gov/vuln/detail/CVE-2021-33813

From the severity of CVE-2021-33813 it looks like this is something that needs to be fixed.

[oosterholt]

A pull request is already created: #188

[hunterhacker]

If you call builder.setExpandEntities(false) then JDOM won't expand entities. The issue here is people may try to set this only via direct calls to the parser and not with the official JDOM solution (which takes precedence).

https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html

I intend to accept the pull request (doesn't hurt, small benefit) but I'm not sure I can push to Maven without either Rolf or a lot of trouble.

[oosterholt]

Any news on this? Did you accept the pull request and did you manage to push to maven?

[hunterhacker]

I accepted. I don't have the maven credentials since that was Rolf's business.

[oosterholt]

Ok. Nice. So, when can we expect a release?

[hunterhacker]

Maybe a non-maven for now?

[memonahad]

@hunterhacker any idea on when we could see an official Maven release with this fix?

[jcoker85]

@hunterhacker Also interested in a Maven release date as soon as possible. Thanks!

[albertus82]

@hunterhacker There are a lot of libs that ultimately depend on jdom2 and now every application that use one of them has CVE-2021-33813, so I hope that a Maven release will follow shortly. Thank you in advance.

[reactivejson]

@hunterhacker we are waiting for a maven release as well, thanks in advance!

[ar]

Hope you can consider the super simple PR #185 as part of the release.

It would make things easier for projects targeting JDK17.

[rolfl]

OK, folk, lots of catching up to do, a mail configuration SNAFU resulted in me missing notifications on JDOM for a while, and I have to collect some content together to push this through (and obviously work to ensure I am not a single point of failure on this process).

Please be patient while I work though all the checks/balances, and ensure all the parts align before pushing a new release.

[rolfl]

Right, merged to master all the changes in the backlog, including a number of fixes and test case changes related to this issue.

Going to get back at this in the next day - will need to address some other issues before releasing a new build still.

[schlm3]

Another ten days later still no release.
What are your plans concerning the release of the security fix?

[ksaiganeshreddy]

when can we expect a release with this security fix??

[swapneel-kore]

I am interested in an update with this security fix as well. Any indication when a release could be coming?

[arvindjagtap8927]

I am also interested in this security fix. When this fix will be released?

[hunterhacker]

I've been waiting for @rolfl to step in here. I'm not exactly sure why he's not. Another email snafu? I tried pinging him privately.

Meanwhile, if you're looking at this issue and are worried about the risks of expanding entities, call the nicely named function setExpandEntities(false) which turns off expanding entities. That always works perfectly.

It's important to note the PR doesn't change any default behaviors. Entities still expand by default. The PR only changes what happens when you don't call the nicely named function but instead talk to the underlying parser directly by setting the obscurely named "http://xml.org/sax/features/external-general-entities" feature to false. If that's what your code does, then might I suggest you change your code to use the nicely named function as well.

I recognize once there's a CVE people want an update. I'm just pointing out the update won't change default behaviors and will only improve the behavior of code that has a calling pattern that isn't robust.

[chadlwilson]

Thanks for helping out @hunterhacker.

To me, it seems the linking of the CVE here, and its current generic description (arguably a bit alarmist?) is creating a slightly misleading impression to folks who are seeing this pop up (possibly from a transitive dependency) that a fix in the library is the only thing required to magically close off a possible XXE attack for people's system.

In reality it seems all the fix does is make the library act according to "principle of least surprise" if a consumer followed the earlier OWASP generic recommendation (prior to this commit) or was doing the generic SAX thing via setFeature.

So the library never prevented anyone from blocking off XXE. One just needed to do it via the setExpandEntities route, not the setFeature route.

So it's a little confusing to me. Seems the earlier OWASP recommendation wasn't tested/verified!? setFeature does have quite a disclaimer also:

jdom/core/src/java/org/jdom2/input/SAXBuilder.java

Lines 778 to 783 in 03a391a

	* NOTE: SAXBuilder requires that some particular features of the SAX parser
	* be set up in certain ways for it to work properly. The list of such
	* features may change in the future. Therefore, the use of this method may
	* cause parsing to break, and even if it doesn't break anything today it
	* might break parsing in a future JDOM version, because what JDOM parsers
	* require may change over time. Use with caution.

If I read correctly, the current plan/merged PR is still dependent on people disabling the functionality with either the builder or the setFeature route, as it doesn't change the behaviour to "secure by default" which would cut off the vulnerability for those who are consuming JDOM transitively via another library. That library would have to have been following the earlier OWASP recommendation in its code, and the behaviour will change. So direct consumers still need to review their code and usages of SAXBuilder, and some of them may not have been doing anything to disable XXE at all in the first place.

My broad summary (please correct me if I am wrong) is

1. if the consuming code makes no attempt to call setFeature("http://xml.org/sax/features/external-general-entities", false) or setExpandEntities(false) then they have a potential XXE attack problem regardless of this CVE (depending on how they receive+use XML documents) making the CVE irrelevant.
2. if the consuming code already called setExpandEntities(false) in relevant places, then they are not affected by this CVE and can safely suppress it.
3. if the consuming code already called setFeature("http://xml.org/sax/features/external-general-entities", false) on its own and expected it to be enough to protect them, then the fix in fix setFeature bug and add test case #188 will magically fix it for them without code change; however they still have the ability to resolve it with setExpandEntities(false) right now.

Am I missing something? I am curious how many real world usages are in the #3 group.

Nevertheless, if we look at this CVE more as

setFeature should work as expected to disable entity expansion

with the broad class of

unintentional security misconfiguration possibly allows unintended XXE

that would seem more accurate to me than

An XXE issue in SAXBuilder in JDOM through 2.0.6 allows attackers to cause a denial of service via a crafted HTTP request

... which does not seem an accurate description of the problem to me, and assumes specific usage context.

[j-be]

@chadlwilson This still doesn't change the fact that (A) the CVE was published over a month ago and (B) the commit fixing the issue (if I am not mistaken) has been there since February and (C) nothing seems to have happened since.

So while I do agree that this fix is not a silver bullet for avoiding XXE, I'm still itchy about how this project handles the issue. They could have just thrown out a half-baked 2.0.6.1 as 2.0.6 + PR#188 advertised as such with just the fix, and do whatever they are doing now for a more polished 2.0.7. So people interested in not having this showing up in their vulnerability scanner could just switch to 2.0.6.1, all other can wait for 2.0.7.

[chadlwilson]

@j-be To be fair, commits were merged a few weeks ago, so that's not "nothing", however It's a bit odd that a quick release hasn't been cut after, I agree. Since the project has had basically no development for over 6 years it seems (as measured by releases), any signs of life are better than nothing.

Nevertheless, the attribution and description of the CVE in this case is a bit dubious IMO. It has created a lot of noise here, amplified by it being on a mature, but old library that is still a common dependency deep down inside a lot of Java software. I feel some more active/mature projects might have had cause to dispute some aspects of the CVE, however no CVE can be looked at blindly without taking into consideration the context of the software's usage. To me, this means context-dependent suppression is a reality of life.

While I agree that it is impossible to assess all ones transitive dependencies, and vulnerability scanner noise is annoying (it's what lead me here also), it doesn't help make our systems more secure to have (what seems to me to be) FUD out there in CVE descriptions and their linked CWE. Let's say a new version is released. I upgrade. The noise goes away, but am I any more secure? Quite likely not. Might make us feel better, but that's not the same as being more secure.

If you don't have the inclination to dig yourself, one suggestion I would have is to look at the next most immediate consuming dependency(ies?) up your chain, and ask for them to assess whether that library is actually vulnerable in its usage of jdom. If not, you can probably suppress and move on. If so, they can follow the suggestions above to mitigate, and then can suppress?

[j-be]

@chadlwilson I agree with most of what you say. But I have to disagree on the "am I more secure".

What I (average developer) tend to do if something pops up on the scanner is: does the update break anything? If not: upgrade. I don't even care about what is or was affected, and if and how it could be exploited in our specific software. As long as a patched version is available and doesn't break anything I upgrade - no questions asked. In the end: chances are I don't even grasp if and how and what the security implications are when directly using a library, leave aside having to check umpteen layers of transitive dependencies for potentially exploitable situations.

The average developer may not even be able to determine this, either because of skill, or simply because assessment means sinking a lot of time into evaluating the issue. Changing a version in pom.xml or build.gradle is done in seconds. If you have a security department in your company it is a different story. But truth is: some of us simply don't. So for some of us the scanners and timely patches is all we have.

With all of this: "Am I more secure?" I'd say "Hell yeah" - not for every individual case, but in long term for sure. I personally simply don't have the time, and for some not even the skill to assess every single vulnerability. And underestimating a single one may be Game Over.

[chadlwilson]

Well, you are taking my words rather out of context by applying them generally, rather than to this specific case as I articulated, so that's not really fair. As a general rule, upgrade first, ask questions later makes sense. I spoke about this specific case, and indicated that maybe it does not.

I did explicitly say "it is impossible to assess all ones transitive dependencies" the equivalent as you, and I also approach upgrading this way. I was just trying to help others, by synthesising what I read across the place, and indicating that in this specific case, it may not affect them if their direct usage is a particular style, and that chasing the devs here because a 3rd party reported a security vulnerability on the basis of "THERE'S A CVE FIX IT FIX IT" and asking them to make the noise go away even if it does little doesn't appear very fair.

Software and supply chain complexity is a big problem for users for sure. It relies on each layer in the chain to be able to translate context to the layer above; to avoid using dead/deprecated dependencies and migrate away; to keep being maintained have have an easy+regular patch process. We don't appear to have a sufficient way of keeping on top of that complexity right now. We have these scanners that cut through the layers, say "oh you have jdom here" and then present users with information that they often don't have the expertise to judge (security department or not) because they are working with a higher level abstraction. They all arrive on Github on the deepest dependency going "fix it fix it", and the middle tiers that actually use that dependency, have necessary context, and may already have a mitigation in place aren't always interrogated for the necessary context. Just reflecting that it doesn't seem to help us very much as either "don't want to go deeper" users of a transitive dependency or "direct" users, who have control of the code to have vagueness in vuln descriptions.

[chadlwilson]

I wonder if it's possible that adding a suffix of .1 is insufficient for some tools to consider it a different version when the prior release doesn't have a matching .0 at the end. That might be considered indeterminate.

Alternatively, Snyk doesn't consider any version fixed right now based on vulnerable versions being listed as versions [0,]. Possibly they require manual review/confirmation. If the latest version of software at time of CVE is known to be vulnerable, there's not really a reason to automatically assume that the next version released actually fixes any given vulnerability, without manual review/checking. Perhaps that's what they do in such cases.

The fact that this ticket is not closed yet possibly won't help either.

OWASP Dependency Check, based on NVD/NIST data does seem to automatically consider 2.0.6.1 as fixed though.

Vulnerable

jdom2-2.0.6.jar | cpe:2.3:a:jdom:jdom:2.0.6:*:*:*:*:*:*:* | pkg:maven/org.jdom/[email protected] | HIGH | CVE-2021-33813

OK

jdom2-2.0.6.1.jar | cpe:2.3:a:jdom:jdom:2.0.6.1:*:*:*:*:*:*:* | pkg:maven/org.jdom/[email protected] | | |

[albertus82]

Now Snyk reports v2.0.6.1 as NOT vulnerable.

[ottlinger]

Any plans to have a release to Maven central? Thanks

[jamesagnew]

@ottlinger the 2.0.6.1 release shows up on maven central for me: https://search.maven.org/artifact/org.jdom/jdom2/2.0.6.1/jar

[ottlinger]

@jamesagnew not sure how long a sync takes, but https://mvnrepository.com/artifact/org.jdom/jdom does not show the newest version yet and I'm unable to fetch with Maven default settings:

[ERROR] Failed to execute goal on project apache-whisker-xml: Could not resolve dependencies for project org.apache.creadur.whisker:apache-whisker-xml:jar:0.2-SNAPSHOT: Could not find artifact org.jdom:jdom:jar:2.0.6.1 in central (https://repo.maven.apache.org/maven2) -> [Help 1]
[ERROR]

[chadlwilson]

@ottlinger Thats because you are looking at the wrong artifact. https://mvnrepository.com/artifact/org.jdom/jdom2

[mmkamburova]

Hello,

Are there any plans to port the fix for CVE-2021-33813 to jdom v1.x? We're experiencing issues with Apache Commons JXPath, which is not compatible with jdom v2.x..

[steinarb]

FWIW the maven-release-plugin still uses JDom v1 (which may cause some strange line ending issues during release https://issues.apache.org/jira/browse/MRELEASE-1025 )

(But maven-release-plugin would IMO be better off replacing JDom 1 with JDom 2, rather than getting a JDom 1 with the security issue fixed... since that would make it possible to fix MRELEASE-1025)

[hunterhacker]

No current plans. If you're worried about entity expansion, call builder.setExpandEntities(false).

The only actual concern is if you (a) intend not to expand entities, (b) don't use the explicit JDOM flag to turn this off but instead (c) directly tell the underlying parser to turn them off, so then (d) you're surprised that your call to the parser didn't apply because JDOM supersedes your complex direct call with its own simple explicit flag setting.

The challenge is once the CVE is out there things are tainted up the chain. I recognize that. And sigh, maybe you can convince me it's worth a chunk of my time. I should really charge support for fixing bugs in software I released 10+ years ago. "The first 10 years are free!" :)

[mmkamburova]

Thanks, @hunterhacker! Well v1.1.3 was released Feb, 2012, so we have some time before the support becomes paid :)
Anyway, I understand your point of view! We're in position where we need to provide a fix for the older releases of our product. Our product is a shared product, which main purpose is to deliver third-party libraries at a shared location. Then the rest of the products in our company use these third-party libraries from that location. And part of our job is to take care of vulnerable libraries by updating them to safer versions.
The actual problem here are the transitive dependencies. In older releases, our products use some old third-party libraries, like Apache JXPath, Apache Velocity and who knows what else, which require the org.jdom package:
"org.apache.velocity 1.5.0.v201212051755) requires 'java.package; org.jdom [1.0.0,2.0.0)' but it could not be found"
In order to port a fix with jdom update from v1.x to v2.x, these products need to do a lot of changes in order to consume Jdom v2.x. Currently we're still struggling with Apache Velocity migration from v1.x to v2.x...

Sorry to bother you again and sorry for taking some of your time! If it's not a lot of work to backport the fix to v1.x, would you consider doing it?

Thanks & Regards,
Maria

[steinarb]

NexusIQ now have the following possible mitigation on its JDOM2 2.0.6 critical warning:

Upgrade to 2.0.6.1
Next version with no policy violation

I.e. this issue is now fixed as far as NexusIQ is concerned.

[Nriver]

It's almost a year passed since the issue started. Wonder if there will be a formal release or not.

[chadlwilson]

@Nriver There is a formal release - 2.0.6.1 (this one). This issue would ideally be closed now to add a bit more clarity to that.

[steinarb]

@chadlwilson I opened this issue, and I'm good with closing this as fixed by 2.0.6.1 now (in case someone was waiting for my input?)

[Nriver]

@Nriver There is a formal release - 2.0.6.1 (this one). This issue would ideally be closed now to add a bit more clarity to that.

Thank you！

[jacalata]

Summing up this thread

• there are no plans to fix this CVE in JDOM 1.x, but I opened a separate issue to track that conversation (Are there any plans to port the fix for CVE-2021-33813 to jdom v1.x? #204 )
• it is fixed (/mitigated) in JDOM2.0.6.1 and there is a formal release through Maven (https://mvnrepository.com/artifact/org.jdom/jdom2/2.0.6.1)

@hunterhacker could you close this one please? :)

[javanobugs]

because
sorry,I used the wrong coordinates。right:jdom2

i also met

[noorharees]

I am Integrating fro jDOM1.X to JDOM2:2.0.6.1

[vmassol]

About:

if the consuming code makes no attempt to call setFeature("http://xml.org/sax/features/external-general-entities", false) or setExpandEntities(false) then they have a potential XXE attack problem regardless of this CVE (depending on how they receive+use XML documents) making the CVE irrelevant.

@chadlwilson Thanks for trying to summarize it! However, does it mean that https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#saxbuilder is not correct when explaining that doing the following is enough to protect from XXE attacks:

SAXBuilder builder = new SAXBuilder();
builder.setFeature("http://apache.org/xml/features/disallow-doctype-decl",true);
Document doc = builder.build(new File(fileName));

This code doesn't use either setFeature("http://xml.org/sax/features/external-general-entities", false) nor setExpandEntities(false) and yet is supposed to be safe from XXE attacks.

Thanks

[chadlwilson]

@vmassol It looks like that change was made 4 months ago in OWASP/CheatSheetSeries@c63fdaf as part of OWASP/CheatSheetSeries#966 so you might want to take it up over there in OWASP land.

Before Feb 2023 it used to look like https://github.com/OWASP/CheatSheetSeries/blob/ef4beb83b59d474d02ec05afa7d79efe554dc3e8/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.md#saxbuilder which is rather different.

[vmassol]

@chadlwilson good point, thanks for your reply!

What they wrote is probably right but it would have been nice to get a confirmation from one of the devs of JDOM2 (I'd expect that before writing the instructions they validated them with the JDOM2 project).