IV falsely detected as not properly generated (reopen #68)

[knewbury01]

Reopening of issue 68

it is unclear to me which versions of CogniCrypt that this issue is expected to be fixed in.
I have been looking at this dataset and from my findings, this is still an issue,

I am able to reproduce the error from the following configuration(s):

Setup:

• get the (built) project in question: git clone https://github.com/anam-dodhy/dragonite-java.git

• using the following few versions of CogniCrypt (note these versions are released after the close date of the original issue):

  • a fairly recent buildable version of CogniCrypt, using the rules from the 2.3 release
  • the corresponding buildable version of CogniCrypt from approx. the 2.3 release, using the rules from the 2.3 release

• Attempted an analysis of both the entire project using this classpath: dragonite-java/dragonite-sdk/build/classes/java/main as well as just a copy of the isolated AESCryptor.class

• Both results in the following in the report:

in Method: byte[] encryptImpl(byte[])
        RequiredPredicateError violating CrySL rule for javax.crypto.spec.IvParameterSpec
            First parameter was not properly generated as randomized
            at statement: specialinvoke $r6.<javax.crypto.spec.IvParameterSpec: void <init>(byte[])>(r16)

Not clear why this issue arises, if it has reappeared, or if the version that it seemed to be fixed in is different than my current work environment in some way.

Please let me know if you need any other details to reproduce,
more details to be found in 68

[smeyer198]

Reporting the error is correct if there is no dataflow between the initialization of the class and the method. Consider the simplified example

public class Example {
    private SecureRandom sr = new SecureRandom();

    public void exampleMethod() {
         byte[] iv = new byte[32];
         sr.nextBytes(iv);
    }

If we analyze this class as it is, CryptoAnalysis considers the class and the method as single entry points, that is, there is no relation of the class field sr and the field sr in the method. This behaviour is correct because there is no dataflow between the call to the constructor SecureRandom and the call to nextBytes. Therefore, the analysis does not that 'sr' in the method is generated securely and assumes it not to be secure, that is, no predicate is ensured on iv. However, if we create a single entry point, e.g.

public static void main(String[] args) {
     Example example = new Example();      // Instantiate 'sr' in this entry point
     example.exampleMethod();              // Use 'sr' in this entry point, i.e. there is a relation
}

then there is no reported error because we have a single entry point where first the field 'sr' is initialized in the constructor of Example which is then used in the method exampleMethod(), that is, there is a relation between the usages in the context/entry point.

[knewbury01]

@smeyer198 quick (naive) question!

re - "CryptoAnalysis considers the class and the method as single entry points"

should the analysis differentiate assuming dataflow/not knowing the entry points based on whether the member is static or not though?

in this example, because exampleMethod is not static, it is not possible to call it without having the constructor assure that SecureRandom sr will be allocated?

or are you saying this is just a known/accepted behaviour of the analysis?

Thanks for the clarification! :)

[smeyer198]

@knewbury01 You are right, one could argue that it may be possible to create contexts like this. However, we use SPDS (Synchronized Pushdown Systems or more precisely IDEal) to determine corresponding seeds and their dataflows. By design, IDEal is strictly flow sensitive, that is, it requires precise dataflows of objects. However, for example, if we take the example from above and add a second method exampleMethod2 that has a call using sr, then IDEal cannot compute precise dataflows because we have the initial class where sr is defined, and two methods that use sr. Without a "main" method, one cannot say whether or which method is called in which order, i.e. there is no clear dataflow. To deal with this problem, IDEal creates single contexts (entry points) for each class initialization and method individually, that is, there is no connection between the objects in the different contexts.

TL;DR The underlying typestate analysis requires precise dataflows to create contexts where all objects are connected. Therefore, we are forced to take this behaviour and it is a design choice in some sence.

I hope that helps to clarify your ideas!