Hi Paul and Alexandru Thank you for the report, Alexandru.
I agree with Paul's analysis: Users of DomToGroovy are by definition trusted parties, and the output should never be executed without human intervention. For example, DomToGroovy's escaping code can be circumvented in several ways, too, making it possible to sneak code from XML into the generated Groovy. Allowing untrusted parties to execute code from XML fed through DomToGroovy would be analogous to leaving your front door open. Allowing XXE is - in that analogy - like leaving the cat-flap unlocked too. Closing off the external entities at parse-time wouldn't hurt, but it is not really a vulnerability in itself. -Jesper > On 28 Aug 2019, at 11.04, Paul King <pa...@asert.com.au> wrote: > > Hi Alexandru, > > [And other developers -- please check my logic and assumptions] > > While Groovy is widely used, I suspect the DomToGroovy tool is not frequently > used. It's typically not used in the context of running arbitrary scripts. > It is typically used by a developer to manually generate some builder-like > Groovy code which they then edit by hand to correctly generate XML > similar to the original XML file. The tool will typically be executed with > the permissions of that developer and if they already have access to > /etc/passwd then I don't find it unexpected for the content to appear. > > Now the real question is whether it can be subverted. If someone had access > to the file system and could write an input file and read an output file and > could run an arbitrary script where they could invoke the DomToGroovy tool, > then it would be possible to exploit the current behavior. But if they can > run an arbitrary script, they could just read the contents of the /etc/passwd > file in this example. I am not sure I see too much of an issue here but > perhaps someone else will chime in if I am missing something obvious. > > I have a vague recollection when we fixed up other tools, we didn't see this > one as being of the same nature as other cases. > But having said that, I don't see any use case where reading external > entities is going to be beneficial for this tool and we could do a similar > thing as we do for other tools, e.g.: > > https://github.com/apache/groovy/blob/master/subprojects/groovy-xml/src/main/java/groovy/xml/DOMBuilder.java#L115-L116 > > <https://github.com/apache/groovy/blob/master/subprojects/groovy-xml/src/main/java/groovy/xml/DOMBuilder.java#L115-L116> > > > I would see this as not being critical but could easily be added for an > upcoming release. > > Let me know if this sounds reasonable or if you see any flaws in my > understanding. > > Cheers, Paul. > > > > <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> > Virus-free. www.avast.com > <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> > <x-msg://5/#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2> > On Wed, Aug 28, 2019 at 7:30 AM Alexandru Gabriel Blanaru > <a.g....@outlook.com <mailto:a.g....@outlook.com>> wrote: > Hello, > > I hope you are well. > > My name is Alexandru and I am currently doing an independent security > research as part of a University project. > > I came across a tool from the Groovy library called DomToGroovy which appears > to have an XXE vulnerability. According to the documentation, it is supposed > to be turning XML content into Groovy scripts but I am not sure whether it is > a feature or an actual vulnerability. The bug is present in the most recent > version of the library according to my findings. > > As proof of concept, a simple payload which makes use of external entities in > order to load and show the contents of "/etc/passwd" was saved in "xxe.xml" > and loaded into the program: > > <?xml version="1.0" encoding="utf-8"?> > <!DOCTYPE foo [ > <!ENTITY file SYSTEM "file:///etc/passwd" > > ]> > <foo> > &file; > </foo> > > I am attaching a screenshot with the output of running DomToGroovy with the > "xxe.xml" file as input. > > The following article does provide information on how to mitigate the > problem. As far as I understand the issue is in one of the overloaded "parse" > functions and the fix should consist of adding some configuration attributes > to the DocumentBuilderFactory instances. > XML External Entity Prevention · OWASP Cheat Sheet Series > <https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html> > > > XML External Entity Prevention · OWASP Cheat Sheet Series > > <https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html> > > The vulnerable code: > > public static Document parse(final Reader input) throws Exception { > DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); > factory.setNamespaceAware(true); > DocumentBuilder builder = factory.newDocumentBuilder(); > return builder.parse(new InputSource(input)); > } > > > Given the fact that this library is widely used I decided to contact you and > not disclose the vulnerability directly (or raise a public ticket on the Jira > Bug tracker). I am willing to provide 3 to 6 months time before full > disclosure, so enough in order to fix, apply patches and inform the user base > to update to the latest version. > > Let me know what you think about this and if you believe the time period > provided is not reasonable. > > Kind regards, > Alexandru > > > <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> > Virus-free. www.avast.com > <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> > <x-msg://5/#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
