[core] security concerns on URL-based rulesets

[bennetyeesc]

Since rulesets can be java class files and there is no mechanism to ensure that URL-based rulesets are loaded only via HTTPS, or that the rulesets are signed or have a certain hash value, it is possible to specify an external ruleset source via HTTP in production environments. This means that DNS poisoning or a compromised web host can inject arbitrary code into the environment where PMD is being used. If PMD is used as part of the developer toolchain and a developer uses a laptop in a cafe, for example, this can lead to the developer's machine being compromised, a RAT installed, and used as a stepping stone when the developer returns to the office environment.
It would be nice if PMD had a flag or configuration where HTTP-based rulesets were rejected, or fancier mechanisms (such as requiring signed code) were implemented.

[jsotuyod]

@bennetyeesc thanks for the report.

The ruleset, if a remote resource, is loaded as a blob, never added to the classpath or even stored on disk, and then parsed as an XML.

Provided the parser is strict enough, that should be safe. We do have some work to do to harden the parsing (reference guide), but shouldn't that be enough for your concerns?

I'm failing to see why a full disable of HTTP resources would be necessary...

[bennetyeesc]

Sorry that I was using terminology poorly. The ruleset is in XML, but it can specify rules that are written in Java. The code uses classloaders to load rules. Maybe my reading of the code was poor, and the URL-based rulesets can only specify classes that are already on the machine, on the classpath? I didn't check, since I figured the functionality of loading a ruleset from an URL was probably designed to allow the web host to fully specify what is run, so the class would also be loaded relative to the URL.

[jsotuyod]

@bennetyeesc yes, rules CAN be written in Java. However, the ruleset can simply reference them by fully-qualified name. You need to make sure the required class is already in the PMD's process classpath on your own to access any Java rules not shipped with PMD.

In Gradle, you would do that by manually adding dependencies under a special pmdClasspath configuration. On Maven, you would have to add dependencies to the plugin configuration on your pom. On ant, you would have to manually add entries to the classpath when doing the taskdef on your build.xml.

I'll double check, just in case, that the classloader used to instantiate classes is not accepting remote resources.... if it does, we may have a real issue beyond the xml parser hardening.

Thanks for the input!

[jsotuyod]

I can confirm the loadClass call is safe. It can in no way reference a resource not already in the classloader (although one CAN specify a http url as a jar for auxclasspath). Non the less, doing so requires altering the contents of the build tool used to run PMD, just like simply setting it as part of the run classpath.

We do have some work to do to harden the XML parsing, but other than that, I see no attack vector on PMD from a ill-intentioned ruleset. If someone does, please provide a proof-of-concept ruleset XML we can work on.

[bennetyeesc]

Thank you for looking into this. I think there is a minor concern still with HTTP rulesets, since an adversary can still cause problems by eliminating rules from the ruleset or causing the "wrong" rules to run. It seems unlikely that a subclass of Rule that would be on the classpath and would be actually dangerous as a stepping stone. Anyway, using HTTP rulesets is arguably just a bad configuration decision (when there may be network-based adversaries).

Thanks again!

[jsotuyod]

True one could tamper on the applied rulesets, but that on itself can at most prevent detection of issues, which would still need to be introduced by an authorized developer. This seems an unlikely attack vector.

The usage of HTTP/HTTPS rulesets on their own is not an issue and several teams / companies use it as a simple way to keep unified rulesets shared across multiple repositories / teams.

Plus, any effective attack on this would require not only affecting a given developer, but also CI servers; and to keep the vulnerabilities over time, to actually affect the whole team for a long enough period of time to allow the team to introduce the vulnerability, discover it, and exploit it.

Shall an organization still be worried about this, they can easily make sure to host the ruleset over https with a proper certificate, or make local copies of the ruleset. I see no reason to modify PMD in any way for such scenarios.

[bennetyeesc]

Agreed. The minor issue is not worth much effort. (I like TLS-everywhere, and having a flag to require HTTPS would be nice [better yet to default secure, and have a flag to allow HTTP], but only as a nice-to-have; not a big deal.)