How to remove this use of dynamic class loading or replace this class loading?

Bhaskar Saikia picture Bhaskar Saikia · Nov 18, 2016 · Viewed 8.1k times · Source
othersMap.put("maskedPan", Class.forName("Some Class"));

Remove this use of dynamic class loading.

Rule

Changelog Classes should not be loaded dynamically Dynamically loaded classes could contain malicious code executed by a static class initializer. I.E. you wouldn't even have to instantiate or explicitly invoke methods on such classes to be vulnerable to an attack. This rule raises an issue for each use of dynamic class loading. Noncompliant Code Example

String className = System.getProperty("messageClassName");
Class clazz = Class.forName(className);  // Noncompliant

See

Answer

Mithfindel picture Mithfindel · Jan 17, 2019

Let's first state the obvious: a SonarQube rule is not meant to be taken as The One And Only Truth In The Universe. It is merely a way to bring your attention to a potentially sensitive piece of code, and up to you to take the appropriate action. If people in your organization force you to abide by SonarQube's rules, then they don't understand the purpose of the tool.

In this case, the rule is telling you that you are at a risk of arbitrary code execution, due to the class name being loaded through a system property, without any safety check whatsoever. And I can only agree with what the rule says.

Now, it is up to you to decide what to do with this information:

  • If you believe that your build and deployment system is robust enough that no malicious code can be side-loaded through this channel, you can just mark this issue as won't fix, optionally provide a comment about why you consider this as not an issue and move on
  • if instead you assume that an attacker could drop a .class or .jar file somewhere in your application's class path and use this as a side-loading channel for arbitrary code execution, you should at the very least validate that the provided class name is one you expect, and reject any unexpected one