Scanning tool whines about possible sec vuln wrt `DOMSerializer`: problem?

[shyam94]

Hi,

Veracode scan reported Medium Vulnerabilities found in jackson-databind-2.13.1.

1. File: com/fasterxml/jackson/databind/ext/DOMSerializer.java Line no: 45 CWE ID: 611 Reason: Improper Restriction of XML External Entity Reference. The product processes an XML document that can contain XML entities with URLs that resolve to documents outside of the intended sphere of control, causing the product to embed incorrect documents into its output. (Please check CWE for more details).

2. File: com/fasterxml/jackson/databind/ext/NioPathDeserializer.java Line no: 53, 58 CWE ID: 73 Reason: External Control of File Name or Path. This call contains a path manipulation flaw. The argument to the function is a filename constructed using untrusted input.

Please let me know if these are false Positives or needs to be addressed.

Thanks, Shyam Joshi

[cowtowncoder]

@shyam94 please explain in detail why you consider these to be valid. Above is not enough to explain the issues.

First, in particular, seems very strange -- I do not follow at all why a serializer -- from a TransformerFactory initalized with

        transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);

would open a security flaw.

The second one could potentially be valid but description does not explain actually how.

But I would really appreciate some more evaluation upfront before filing issues -- typically it should "this looks like a security flaw based on X, Y and Z" instead of "hey there might be something please go spend time investigating".

Aside from that, there is an invitation-only google group:

https://groups.google.com/g/jackson-dev-infosec

that if you are interested I could invite you (or anyone else in); this might be a good place for discussions for security-sensitive issues.

[cowtowncoder]

Oh. And of course there was this:

Veracode scan reported 

Going forward, please do not send plain scans without at least brief evaluation. I dislike this current crop of security tools in general (not necessarily this one in particular :) ), and question vendors' motives in spreading FUD to drum up business. I've yet to see security scan tool that has low enough false positive rate -- and writing one may be impossible, in general -- where it would be ok to simple pass their findings. So chances of anything they report to be bogus appears to be at least 50% or higher.

So. While I do appreciate reports of vulns, I do have many bad experiences with such scans getting reported with lots of noise.

Apologies for a rant; this happens to be one of few major pet peeves I deeply care about.

[pjfanning]

@cowtowncoder the static code analysis tools often expect code to completely disable DTD parsing.

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

TransformerFactory tf = TransformerFactory.newInstance();
tf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
tf.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");

The existing code might be enough as it enforces limits so that DTDs can be used but any harmful expansions will result in errors.

   transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);

[cowtowncoder]

@pjfanning So basically it could be tool malfunction or Operator Error -- that is, submitter really needs to do basic validation of potential concerns tool reports as tools often report false positives. What I mean by this is that there are multiple ways to disable various features and what matters is whether there is a hole or not; and not whether scanning tool is happy.

In this case what would be needed, really, is a reproduction of actual vuln. Reporter should also (IMO) have done basic due diligence instead of dumping "hey you go check something for me" request on maintainers' lap.

Will label as "need unit test" and close so someone interested may (if they choose to) check it out.

[cowtowncoder]

Btw, I am not necessarily opposed to additional TransformerFactory set up changes; PRs welcome (@pjfanning if you think that makes sense?). Above rant is more about meta-issue I have observed a few times.

[pjfanning]

@cowtowncoder I agree that a test case that proves there is a real issue would be the best starting point. I was just highlighting the problem with static code analysers - the current code should be secure as is.

[cowtowncoder]

@pjfanning yes, I appreciate that & thank you for context!

But I also realized that there is the balance between minimal (but known secure) and somewhat redundant checks that keep tools happier. I try to be pragmatic so ... open to suggestions on modest additions as necessary. :)

[cowtowncoder]

I don't think there are specific work items here: but if some are found, please file a new issue (with reference to this one) with details.

[tarekahf]

I am getting the same flaws reported by the scan tool:

Improper Restriction of XML External Entity Reference (CWE ID 611)
jackson-databind-2.13.0-rc1.jar   - com/.../ext/DOMSerializer.java 45
woodstox-core-6.2.4.jar  -  com/.../reader/GrammarReader.java 526
woodstox-core-6.2.4.jar  -  com/.../WSDLSchemaReader.java 118

It's not clear to me if the issue was resolved as per this post.

I got the same flaw in my code, and I was able to resolve it. But how can I handle this flaw in the jackson-databind dependency? Is there anything I should do in my code to prevent such flaws to be reported?

[pjfanning]

try latest version of woodstox - https://mvnrepository.com/artifact/com.fasterxml.woodstox/woodstox-core

also worth trying a newer jackson-databind (eg 2.14.2 or 2.15.0-rc1)

[tarekahf]

Thank you @pjfanning

I will update the woodstox dependency and try again.

In the meantime, I wanted to list the other flaws reported that I need to resolve also:

External Control of File Name or Path (CWE ID 73)
woodstox-core-6.2.4.jar  - com/.../wstx/util/URLUtil.java 49
woodstox-core-6.2.4.jar  - com/.../wstx/util/URLUtil.java 85
woodstox-core-6.2.4.jar  - com/.../wstx/util/URLUtil.java 155
stax2-api-4.2.1.jar      -.../XMLValidationSchemaFactory.java 153

Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection') (CWE ID470)
woodstox-core-6.2.4.jar  -  .../DatatypeLibraryLoader$Service$Loader.java 1
woodstox-core-6.2.4.jar  -  .../DatatypeLibraryLoader$Service$Loader2.java 1
woodstox-core-6.2.4.jar  -  com/.../VerifierFactory.java 157
stax2-api-4.2.1.jar   -  .../XMLValidationSchemaFactory.java 302
stax2-api-4.2.1.jar   -  .../XMLValidationSchemaFactory.java304

[pjfanning]

Could you use woodstox project to talk about woodstox?

[cowtowncoder]

Also note that jackson-databind itself does NOT use Woodstox (or Stax API in general): DOM implementation would come from JDK (uses Xerces).

Only jackson-dataformat-xml depends on Woodstox. So going forward that'd be the issue tracker.

I realize that CVE/Vuln maintainers are often confused about things and may mislabel jackson-databind as affected, for some reason. But that's no reason for us to be confused.

[tarekahf]

Could you use woodstox project to talk about woodstox?

I created an issue on woodstox project:

https://github.com/FasterXML/woodstox/issues/169#issue-1636827411

[tarekahf]

I pulled in the latest jackson-databind version 2.15.0-rc1 and I am still getting exactly the same XXE Attach vulnerability with this JAR.

Any ideas on how to resolve this?

[cowtowncoder]

@tarekahf You or someone else needs to do some digging: library authors often have limited time and interest to decipher what various security tools report or suspected problems -- majority of these things flagged may not be actual vulnerabilities at all (based on my past experiences).

[pjfanning]

We have merged an extra change to 2.15.0-rc2. still in snapshot.

[tarekahf]

We have merged an extra change to 2.15.0-rc2. still in snapshot.

@pjfanning thank you. I will look for this latest update. For the time being, I ignored such vulnerabilites since they are not in my code.

I am thinking ... how would a user of my code (the client) take advantage of such XXE attack vulnerabilities present in say jackson-databind (as the tool claimed)? I can't wrap my head around such a scenario. I can understand that the client may take advantage of my code, not the code I depend on. For XXE to occur, the client must submit and XML file, and my code already mitigated such vulnerabilities.

Please let me know your thoughts as I need to verify my understanding to be able to engage in discussions when talking about this issue.

See this for details: https://stackoverflow.com/a/75825877/4180447

[cowtowncoder]

XXEs via DTDs typically allow embedding of external files as part of document being processed. Same probably applies to XSL(T) (stylesheets). So if taking caller's XML document, processing, it could extract information from other resources on server, and (depending on how output is handled) get it back.

With DTDs these are via external parsed entities, so information to get also has to be in XML. Possibly not limited as much with XSL (but needs to be textual).