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