Skip to content
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

Closed
krabbenpuler opened this issue Jul 22, 2019 · 35 comments
Closed

Security: XXE in initDocumentParser #467

krabbenpuler opened this issue Jul 22, 2019 · 35 comments

Comments

@krabbenpuler
Copy link

krabbenpuler commented Jul 22, 2019

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.

@jgallimore
Copy link
Contributor

@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?

@krabbenpuler
Copy link
Author

krabbenpuler commented Aug 5, 2019

@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>

@jgallimore
Copy link
Contributor

That's great - thanks for that.

jgallimore added a commit to jgallimore/quartz that referenced this issue Aug 6, 2019
@jgallimore
Copy link
Contributor

PR for master: #472

@jgallimore
Copy link
Contributor

I'll send an email, but I'm happy to file a CLA if needed.

jgallimore added a commit to jgallimore/quartz that referenced this issue Aug 6, 2019
@jgallimore
Copy link
Contributor

Same patch for the quartz-2.2.x branch. #473

@ddillard
Copy link

ddillard commented Aug 7, 2019

@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?

@krabbenpuler
Copy link
Author

@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.

@julyhurt
Copy link

@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?

@jgallimore
Copy link
Contributor

@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.

@onlysyz
Copy link

onlysyz commented Aug 15, 2019

what a sad story~~~

jgallimore added a commit to jgallimore/quartz that referenced this issue Aug 16, 2019
jgallimore added a commit to jgallimore/quartz that referenced this issue Aug 16, 2019
jgallimore added a commit to tomitribe/quartz that referenced this issue Aug 16, 2019
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
jgallimore added a commit to tomitribe/quartz that referenced this issue Aug 16, 2019
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
jgallimore added a commit to tomitribe/quartz that referenced this issue Aug 16, 2019
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
@vineetpandey
Copy link

vineetpandey commented Aug 26, 2019

Does this vulnerability impacts quartz-2.3.1 as well ??
If yes, did you committed a patch for it?

jgallimore added a commit to tomitribe/quartz that referenced this issue Aug 28, 2019
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
jgallimore added a commit to tomitribe/quartz that referenced this issue Aug 28, 2019
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
jgallimore added a commit to tomitribe/quartz that referenced this issue Aug 28, 2019
Issue quartz-scheduler#467 provide XML parser with a strong configuration to prevent …
@jgallimore
Copy link
Contributor

@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.

@jgallimore
Copy link
Contributor

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)
https://oss.sonatype.org/content/repositories/orgtomitribe-1072 -> 2.3.2 (source: https://github.com/tomitribe/quartz/tree/quartz-2.3.2)

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 org.tomitribe.quartz as opposed to org.quartz-scheduler, so your dependency would need to be

    <dependency>
      <groupId>org.tomitribe.quartz</groupId>
      <artifactId>quartz</artifactId>
      <version>${quartz.version}</version>
    </dependency>

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.

@krabbenpuler
Copy link
Author

@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!

@melloware
Copy link
Contributor

melloware commented Sep 3, 2019

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?

@davidmoten
Copy link
Contributor

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.

@jgallimore
Copy link
Contributor

  • What's put in the S3 bucket to set the times?
  • How is the bucket secured?
  • Do you trust the user and their input?

If they're providing XML that is parsed with a parser configured in initDocumentParser() then it is potentially vulnerable.

@CiaranLMurphy
Copy link

@jgallimore Thanks for the help!

@jgallimore
Copy link
Contributor

@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

@melloware
Copy link
Contributor

@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?

@vineetpandey
Copy link

@jgallimore Thanks for the resolution. Much appreciated.
Any chance if this quartz fix can get Terracotta published ?

@jgallimore
Copy link
Contributor

You're very welcome, and I appreciate the reviews, feedback and comments from everyone here.
I also would like to see a Terracotta version published, but I haven't seen any activity on this project from its committers for something like 7 months, so I'm not sure what will happen.

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.

@vineetpandey
Copy link

@zemian - you are one of the significant contributor of quartz.
Can you get this Terracotta published or redirect to correct person?

@melloware
Copy link
Contributor

@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.

When you lose interest in a program, your last duty to it is to hand it off to a competent successor.
-The Cathedral and the Bazaar

@chrisdennis
Copy link
Contributor

@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.

@melloware
Copy link
Contributor

Much appreciated @chrisdennis

chrisdennis pushed a commit to chrisdennis/quartz that referenced this issue Oct 4, 2019
@mtarnawa
Copy link

mtarnawa commented Oct 11, 2019

@chrisdennis - any chance of this fix getting an incremental release in Maven?

@melloware
Copy link
Contributor

@mtarnawa He is working on it. They are planning on getting a 2.3.2 in Maven with these security fixes.

@chrisdennis
Copy link
Contributor

Just want to get #475 merged in and then I'll move forward with releasing. Looking like that will happen next week now.

@vineetpandey
Copy link

Thanks @chrisdennis, @melloware, @jgallimore!
Much appreciated your efforts.

Looking forward to the official release.

@flowergoo
Copy link

@chrisdennis thanks for the contribution, this security vulnerability has been there for a while. May I know when you plan for the release? :)

@melloware
Copy link
Contributor

melloware commented Oct 24, 2019

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests