False Negative: Weak Hashing Spec from Object to be used with MessageDigest is not reported

Clean Code Report False-positive / False-negative...
, ,
6

Hey there Amit, thank you for pointing this out. Currently our analyzer avoids evaluating non-constant identifiers. This is because evaluating something which is not final might raise a lot of false positives.
To give you an example on top of your example

String algorithmName = "MD5";
try {
    someUknownMethodThatEditsStrings(algorithmName);
    digest = MessageDigest.getInstance(algorithmName);
    System.out.println(digest.getAlgorithm());
}

algorithmName not being final makes it very risky to assume its possible value if there are unknown operations before it gets passed to the MessageDigest, and we always prioritize not raising FPs.

This being said, we might still be able to statically determine if a non-constant variable has gone through steps in the code that might have changed its value. I think it would be interesting to try and track the lifecycle of the variable and determine if it has been changed or not, in order to consider it just like a constant. I created a ticket to dig into it!

2 Likes