Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cryptographic APIs misuses #898

Open
misterAnderson90 opened this issue Feb 23, 2022 · 3 comments
Open

Cryptographic APIs misuses #898

misterAnderson90 opened this issue Feb 23, 2022 · 3 comments
Assignees
Labels

Comments

@misterAnderson90
Copy link

I'm a PhD student interested in finding security vulnerabilities in open source projects.

We found a total of 180 warnings (indicating potential vulnerabilities) when running the CogniCrypt static analyzer (*) on dash-wallet (or its library dependencies). We documented each one of these issues in private gists for the sake of confidentiality (non-disclosure).

Can you please let us know whether we can share these gists with you? We are eager to evaluate the perception of developers (e.g. severity of these warnings) and improve dash-wallet's security, and the quality of the reports of static analysis tools.
(*) https://github.com/CROSSINGTUD/CryptoAnalysis

@HashEngineering
Copy link
Collaborator

This tool does look interesting. For me it gives stack overflow errors or out of memory errors.

@HashEngineering HashEngineering self-assigned this Feb 24, 2022
@misterAnderson90
Copy link
Author

Hello @HashEngineering,

Depending on the program size it requires a lot of memory to complete the analysis. At Dash-wallet, I needed 30 GB of ram.
Are you interested in receiving these warnings reports? If yes, how can I share them with you?

@HashEngineering
Copy link
Collaborator

I changed the RAM parameters and was able to run the report. It will take some time to go through the findings.

I suppose that the rules in the system are looking for insecure code.

One example is this:

Findings in Java Class: org.bitcoinj.script.Script

         in Method: void executeScript(org.bitcoinj.core.Transaction,long,org.bitcoinj.script.Script,java.util.LinkedList,java.util.Set)
                ConstraintError violating CrySL rule for java.security.MessageDigest (on Object #4679457ec2309e70f0138dea44d0aac0793412778b328d5792e0d2a285939f76)
                        First parameter (with value "SHA-1") should be any of {SHA-256, SHA-384, SHA-512, SHA-512/256, BLAKE2B-256, BLAKE2B-384, BLAKE2B-512, BLAKE2S-256, KECCAK-256, KECCAK-384, KECCAK-512, WHIRLPOOL}
                        at statement: $r18 = staticinvoke <java.security.MessageDigest: java.security.MessageDigest getInstance(java.lang.String)>(varReplacer58484)

Which if we get the source in dashj - we find this (originally is taken from the Bitcoin standard).

                case OP_SHA1:
                    if (stack.size() < 1)
                        throw new ScriptException(ScriptError.SCRIPT_ERR_INVALID_STACK_OPERATION, "Attempted OP_SHA1 on an empty stack");
                    try {
                        stack.add(MessageDigest.getInstance("SHA-1").digest(stack.pollLast()));
                    } catch (NoSuchAlgorithmException e) {
                        throw new RuntimeException(e);  // Cannot happen.
                    }
                    break;

The OP_SHA1 is an opcode for the Dash script language. Most likely it is no longer used. I suppose we could work on getting this opcode made obsolete or deprecated. But that is outside the scope of my work which is to implement the Dash standard in Java.

Other items are in dependencies and further investigation would be required to determine if an update of such dependencies would eliminate the warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants