CROSSINGTUD / CryptoAnalysis

CogniCrypt_SAST: CrySL-to-Static Analysis Compiler
Eclipse Public License 2.0
64 stars 39 forks source link

Weak Algorithm Name as String Parameter not detected after Transformation #269

Closed LordAmit closed 1 month ago

LordAmit commented 4 years ago

Hi,

My team is conducting academic research on Java Cryptography API based misuse using your tool. We found that we could not detect some potential cryptographic misuses.

We believe this may be due to underlying implementation or design gaps. Each cryptographic vulnerability was generated as a barebone Java project that only contained a single vulnerability in the main function and used up to two java source files. A jar was made which was then scanned.

Additionally, all cryptographic API calls were from Java Cryptographic Architecture (JCA).

Problem

Replacing a Secure Parameter with an Insecure Parameter:

MessageDigest.getInstance("SHA-256".replace("SHA-256", "MD5"));

Replacing an Insecure Parameter with an Insecure Parameter:

Cipher.getInstance("AES".replace("A", "D"));

where "AES" by itself is insecure as it defaults to using ECB.

Transforming string case, e.g., from lower to upper case:

Cipher.getInstance("des".toUpperCase(Locale.English));

Replacing a noisy version of insecure parameters:

Cipher.getInstance("DE$S".replace("$", ""));

Environment

Component Version
Java Runtime OpenJDK version 1.8.0_232 64 bit
CogniCrypt version used CryptoAnalysis-2.0-jar-with-dependencies.jar

I understand that the current release snapshot is version 2.7. However, when we started using it - the recommended release was 2.0 in readme.md file. I hope this report will be useful for making your tool (including the current combined version) even better.

Please let me know if you need any additional information (e.g., logs from our side) for fixing these issues.

Thanks again!

johspaeth commented 4 years ago

Thanks for reporting.

Those transformation are indeed not yet handled by CryptoAnalysis. We have not found such transformation to appear in practice (we did not analyze malware!), and we did not further spend time on it.

Do you have concrete use cases that require such transformation?

I am happy to provide you pointers in the implementation to where one has to model the transformation.

LordAmit commented 4 years ago

Hi!

Thanks for getting back. These transformations can happen in several, non-malicious scenarios. For example, a string can be converted to a different case by a novice developer who is trying to follow naming conventions. Or a developer can replace characters that he added for his own convenience before passing it on as parameter.

Here is an example from a github repository that shows such behavior:

String algsPadd = (algs.name() + "/" + mode.name() + "/" + padd.name()).replace('_', '-');
Cipher cipher = Cipher.getInstance(algsPadd);

Of course, it does not match exactly with the examples I provided, but it is similar.

We also noticed that other similar tools for crypto API misuse detection, such as Xanitizer, catch such transformations.

If ignoring such transformations is a design choice, I request that such design choices be mentioned in documentation (readme/wiki).

smeyer198 commented 1 month ago

The mentioned transformations were added in #683. Further transformations may be added in issue 687