-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Security: XXE in initDocumentParser #467
Comments
@krabbenpuler I've been taking a look at this, and I think I have a patch I can send as a PR for master. Do you have a specific test case I can verify it with? |
@jgallimore Sure, the following attack vector worked in my case: <?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE foo [<!ELEMENT foo ANY >
<!ENTITY xxe SYSTEM "/" >]>
<job-scheduling-data xmlns="http://www.quartz-scheduler.org/xml/JobSchedulingData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.quartz-scheduler.org/xml/JobSchedulingData http://www.quartz-scheduler.org/xml/job_scheduling_data_2_0.xsd" version="2.0">
<schedule>
<job>
<name>xxe</name>
<group>native</group>
<description>&xxe;</description>
<job-class>org.quartz.jobs.NativeJob</job-class>
<durability>true</durability>
<recover>false</recover>
</job>
</schedule>
</job-scheduling-data> |
That's great - thanks for that. |
…tion to prevent XXE attacks
PR for master: #472 |
I'll send an email, but I'm happy to file a CLA if needed. |
…tion to prevent XXE attacks
Same patch for the quartz-2.2.x branch. #473 |
@krabbenpuler You said you got the CVE, why are the versions in the CVE up to and including 2.3.0 instead of 2.3.1? |
@ddillard Good point, I'm actually not sure why only the 2.3.0 made it into the CVE. I'll inform MITRE about that as soon as possible. |
@jgallimore How can we avoid this issue in current 2.3.0 version without your patch? Any workaround or setting in the qutarz configuration file can avoid this issue? |
@julyhurt I think it depends on how you're using and driving Quartz. For context, I'm coming from Apache TomEE, where we have an old version of TomEE that uses Quartz for EJB timers, so we're using it as a library, rather than standalone. Its possible that our use of Quartz isn't actually vulnerable, but this issue flagged up on a scan, so I'm more than happy to help fix it. In terms of mitigating this without a patch - if you're passing XML to Quartz, and that XML is untrusted, I would do something to check it before handing it over to Quartz - so that might mean parsing it with a well configured parser first, and then passing it to Quartz. There's no 2.3.x branch, but I'm happy to patch 2.3.0 and create a branch for that. I'm not a committer on Quartz though, so I can't actually get anything into the repository, or get a release made. I have tried posting to the user and dev forums, both of which seem to have disappeared (for me at least) and I've tried emailing SoftwareAG to see if they need a signed CLA, per the guidelines here: https://github.com/quartz-scheduler/quartz/blob/master/docs/contribute.adoc. I've heard nothing back, so I'm starting to be a little concerned that potentially no-one is working on the project any more. |
what a sad story~~~ |
…tion to prevent XXE attacks
…tion to prevent XXE attacks
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
Does this vulnerability impacts quartz-2.3.1 as well ?? |
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
@vineetpandey It does affect 2.3.1. There's no 2.3.x branch for me to create a PR against. There doesn't seem to be much movement here, but it seems like there are folks interested in patches, so I'm trying to push some new binaries so people can try them out. I would create a 2.3.x as part of that. |
As there hasn't been any activity on this for over a month now, I have forked this repository, and created builds for 2.2.x and 2.3.x, and have staging repositories for both on oss.sonatype.org (these would go on to Maven central if promoted): https://oss.sonatype.org/content/repositories/orgtomitribe-1071/ -> 2.2.4 (source: https://github.com/tomitribe/quartz/tree/quartz-2.2.4) I'm doing some testing on these here, but I'd be grateful if anyone else can try too (@krabbenpuler, maybe you could try your tests against these to see if my patch holds up) Please note that the groupId for these artifacts is
I don't know how long the staging repositories will be kept on oss.sonatype.org for, but I suspect its 14 days. We'd need to promote or drop them in that time. |
@jgallimore a colleague of mine will test your build soon since I'm currently out of office. Thanks for your effort though, glad to see that some people are willing to work on that issue! |
I am actually surprised Terracotta has not jumped on this since CVE's are a high priority across the industry now. @jgallimore much appreciated your work hopefully your PR will be accepted and an official release will be done by the Terracotta team. @zemian are you still working on this project? |
So the mechanism for the vulnerability to be exploited is someone allowing their quartz to be configured by arbitrary submissions from a client? Seems like a bizarre use-case but happy for the vulnerability to be fixed. Doesn't strike me as urgent to fix but I'd also comment that I like to see projects releasing often with minor fixes as well. |
If they're providing XML that is parsed with a parser configured in initDocumentParser() then it is potentially vulnerable. |
@jgallimore Thanks for the help! |
@CiaranLMurphy You're welcome! Your use of CronScheduleBuilder sounds ok, but obviously everyone needs to make their own assessments, as there are different ways of using Quartz. I can't guarantee that a specific use-case is "fine" or not. Checking whether this method is called in your system is a start: https://github.com/quartz-scheduler/quartz/blob/master/quartz-core/src/main/java/org/quartz/xml/XMLSchedulingDataProcessor.java#L166 |
@chrisdennis I know you mostly work on Ehcache but since you are a member of Terracotta can you get this critical CVE merged and Quartz published? or know who to contact to get it published? |
@jgallimore Thanks for the resolution. Much appreciated. |
You're very welcome, and I appreciate the reviews, feedback and comments from everyone here. I did publish my binaries (under a org.tomitribe.quartz groupId) on Maven central in case anyone wishes to use them: https://mvnrepository.com/search?q=org.tomitribe.quartz. Corresponding source code is here: https://github.com/tomitribe/quartz/tree/quartz-2.2.4 and here: https://github.com/tomitribe/quartz/tree/quartz-2.3.2. |
@zemian - you are one of the significant contributor of quartz. |
@vineetpandey I have been trying to contact @zemian and @chrisdennis they are both active TerraCotta commiters for Quartz and Ehcache and it has not gotten any response.
|
@melloware I'm working on trying to get a future plan for Quartz figured out, but this is something that will have to happen on its own timeframe - it's likely going to involve a lot of moving parts, many of which I have little to no influence over. Once I have the beginning of a plan figured out I'll likely file an issue for it here. In the meantime I'll look in to whether I can independently get this specific issue released on a shorter timeframe. |
Much appreciated @chrisdennis |
…tion to prevent XXE attacks
@chrisdennis - any chance of this fix getting an incremental release in Maven? |
@mtarnawa He is working on it. They are planning on getting a 2.3.2 in Maven with these security fixes. |
Just want to get #475 merged in and then I'll move forward with releasing. Looking like that will happen next week now. |
Thanks @chrisdennis, @melloware, @jgallimore! Looking forward to the official release. |
@chrisdennis thanks for the contribution, this security vulnerability has been there for a while. May I know when you plan for the release? :) |
@flowergoo It was released yesterday as 2.3.2 and is available on Maven Central. https://github.com/quartz-scheduler/quartz/releases/tag/v2.3.2 |
The method initDocumentParser() in the XMLSchedulingDataProcessor.java does not forbid DTDs, which allows a context-dependend attacker to perfom an XXE. The vulnerability is confirmed working in version 2.2.3.
I reported this issue to the MITRE cooperation already which assigned the identifier CVE-2019-13990 to this vulnerability. Please confirm the existence of this vulnerability so that the CVE-entry can be completed and published.
The OWASP Foundation provides an XXE Prevention CheatSheet that explains in detail the steps to prevent this security issue.
Credits to Andreas Schoedl for working out the relevant code section affected by this issue.
The text was updated successfully, but these errors were encountered: