-
Notifications
You must be signed in to change notification settings - Fork 39
False Positive, False Negatives & True Positives
A false positive is an issue that doesn’t actually exist in the code. It doesn’t need to be fixed. This happens when no rule violation exists, but our analyzer still reports an error.
public class MessageDigestExample {
public static void main(String[] args) throws NoSuchAlgorithmException {
// TODO Auto-generated method stub
MessageDigest md = MessageDigest.getInstance("SHA-256");
byte[] ba = new byte['c'];
for(int i=0; i<1; i++) {
md.update(ba);
}
byte[] result = md.digest();
}
}
For the above sample code, the for
or while
loop is represented as if-else
in jimple code as follows.
label0:
if i < 1 goto label1;
goto label2;
label1:
...
virtualinvoke md.<java.security.MessageDigest: void update(byte[])>(ba);
...
goto label0;
label2:
...
temp$3 = virtualinvoke md.<java.security.MessageDigest: byte[] digest()>();
...
Hence there exists a path to digest()
without a call to update()
when i < 1
. This results in a TypeStateError since CrySL rule for MessageDigest has the following ORDER getInstance() -> update() -> digest()
.
Findings in Java Class: crypto.MessageDigestExample
in Method: void main(java.lang.String[])
TypestateError violating CrySL rule for java.security.MessageDigest (on Object #85a17fcc58b0869d4a80d7c1a778054d3ec918fbfcbab0fb0eed2d7191ca9374)
Unexpected call to method <java.security.MessageDigest: byte[] digest()> on object of type java.security.MessageDigest. Expect a call to one of the following methods <java.security.MessageDigest: void update(byte[])>,<java.security.MessageDigest: void update(byte[],int,int)>,<java.security.MessageDigest: byte[] digest(byte[])>,<java.security.MessageDigest: void update(java.nio.ByteBuffer)>,<java.security.MessageDigest: void update(byte)>
at statement: r3 = virtualinvoke r1.<java.security.MessageDigest: byte[] digest()>()
public byte[] of(InputStream inputStream) throws NoSuchAlgorithmException, IOException {
MessageDigest md = MessageDigest.getInstance("SHA-256");
byte[] ba = new byte['c'];
for(;;) {
int n = inputStream.read(ba);
if(n == -1) {
break;
}
md.update(ba);
}
byte[] result = md.digest();
return result;
}
In the above sample code, there is a break
before update
call. Therefore there exists a path from getInstance()
to digest()
without the update()
method (when n==-1
). This violates the ORDER rule for MessageDigest. Hence the analysis reports TypestateError.
in Method: byte[] of(java.io.InputStream)
TypestateError violating CrySL rule for MessageDigest (on Object #52592a2e8b16becdacf2430edb7200f18925649b98f438e1ca846225a26d4a2f)
Unexpected call to method <java.security.MessageDigest: byte[] digest()> on object of type java.security.MessageDigest. Expect a call to one of the following methods <java.security.MessageDigest: void update(byte[])>,<java.security.MessageDigest: void update(byte[],int,int)>,<java.security.MessageDigest: byte[] digest(byte[])>,<java.security.MessageDigest: void update(java.nio.ByteBuffer)>,<java.security.MessageDigest: void update(byte)>
at statement: r4 = virtualinvoke r2.<java.security.MessageDigest: byte[] digest()>()
private static String getSHA256(InputStream uri) throws IOException, NoSuchAlgorithmException
{
InputStream is = uri;
MessageDigest md = MessageDigest.getInstance("SHA-256");
DigestInputStream dis = new DigestInputStream(is, md);
while (dis.read() != -1) {
// we just drain the stream here to update the Message Digest
}
md = dis.getMessageDigest();
StringBuilder sb = new StringBuilder(64); // SHA-256 is always 64 hex characters
for (byte b : md.digest())
{
sb.append(String.format("%02x", b));
}
return sb.toString();
}
In the sample code above, the two objects DigestInputStream and MessageDigest are used correctly and securely, but a TypestateError is shown. This is because the MessageDigest object is initialized and the digest()
call is done in the end. The error shows that a call update()
must be done between the two previous calls. The call update()
is in fact done internally by the call read()
of object DigestInputStream, which updates the MessageDigest every time an InputStream is read. Since the two rules don't know about the existence of each other, they are in a way crashing with the ORDER section of one another and showing a False Positive.
A false negative is an issue that goes undetected. This happens when a rule violation exists, but our analyzer doesn't report any error.
A true positive is an issue that actually exists in the code. It needs to be fixed. This happens when a rule violation exists and our analyzer reports it.