10
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

Bhaskar Saikia
  • 172
  • 1
  • 2
  • 15

3 Answers3

10

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
Mithfindel
  • 4,553
  • 1
  • 23
  • 32
7

One option would be something like that:

Class<?> cls;

switch (System.getProperty("messageClassName")){
   case "com.example.Message1":
     cls = com.example.Message1.class;
     break;
...
}
k5_
  • 5,450
  • 2
  • 19
  • 27
  • It is working or not? @k5_ what is I dont know class name. It is provided me by user and there are 100 of classes. – Nitul Mar 01 '18 at 05:50
4

Well you could try to outsmart the Sonar rule, e.g. by using reflection to call the Class.forName() method, but I'm feeling you would be solving the wrong problem there:

Class.class.getDeclaredMethod("forName", String.class).invoke(null, className);

The right way to do it is to either convince the people who run Sonar in your org that what you do is necessary and they need to make an exception to the rule for you. Or if you can't convince them, stop doing it.

Sean Patrick Floyd
  • 292,901
  • 67
  • 465
  • 588
  • Will try and let you know SONAR is now detecting or not, Here the className at last is this? : `String className = System.getProperty("messageClassName");` – Bhaskar Saikia Nov 18 '16 at 22:56
  • 1
    why you are using `String.class` can you please explain you answer little bit?How this will replace my `class.forname` Please. – Bhaskar Saikia Nov 19 '16 at 06:38