Existing rule Dynamically executing code is security-sensitive is missing some reflection scenarios

Clean Code New rules / language support
,
1

Dynamically executing code is security-sensitive squid:S1523 seems to be missing reflection initiated from java.beans.PropertyDescriptor.
We use SonarQube Community Edition Version 7.7 (build 23042).
/Anders

2

Hi @Anders,

Thank you for your feedback.
Could you give a few examples of sensitive code so that I can understand the use case ?

Regards.
Pierre-Loup

3

Hi Pierre-Loup,

Here is an extaction of the code that should flag a reflection hot spot.

import java.beans.PropertyDescriptor;

public void addExtensionTrigger(ExtensionTrigger extensionTrigger, ValidationType... validationTypes) {
  for(ValidationType validationType : validationTypes) {
    try {
      Object value = new PropertyDescriptor(validationType.getPropertyName(), ExtensionTrigger.class).getReadMethod().invoke(extensionTrigger);
      if (value == null) {
        validationResults.add(validationType);
        continue;
      }
      targets.add(new ValidationTarget(validationType, value));
    } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException | IntrospectionException e) {
      LOGGER.error(MessageFormat.format("Failed to add extensionTriger = {0} with ValidationType = {1}", extensionTrigger, validationType), e);
      targets = null;
      return;
    }
  }
}

/Anders

4

Hi @Anders ,

Thank you for this code sample.
From what I see you expect to have the java.beans.PropertyDescriptor.getReadMethod() method added to the other Java reflection methods that are already found by rule S1523.

We can’t do it as this rule have been dropped lately when we decides to elevated Security Hotspots to first-class citizen status. Raising security hotpots on reflection methods was generating too much noise, so we felt that it was bringing more trouble than real value to the developer.

If you think we should have kept this rule, please give use your feedback on this thread. You could, for instance, detail what is the underlying security risk in the code sample you provided.

Regards.
Pierre-Loup

2 Likes