-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Crypto: Add Java Cryptographic Analysis Queries #20605
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
Draft
bdrodes
wants to merge
31
commits into
github:main
Choose a base branch
from
bdrodes:santander-java-crypto-check
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 5 commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
c5cf0ff
added java cryptographic check queries
unprovable f38ab45
removed all @security.severity ratings to keep the main impartial
unprovable bba541c
Merge remote-tracking branch 'upstream/java-crypto-check' into santan…
bdrodes cf88e3f
Crypto: Standardize naming where use of "family" and "type" have been…
bdrodes 1b1b333
Crypto: Modify suggested queries per misc. side conversations on stan…
bdrodes 143be8c
Crypto: Remove redundant queries.
bdrodes bd34b6c
Crypto: Removing JCA model of random, need to reassess this as this i…
bdrodes 83ff70b
Crypto: Adding tests for insecure iv or nonce. Updating generic liter…
bdrodes 8e10e19
Crypto: Adding query for unknown IV initialization.
bdrodes 75b5a9f
Crypto: Update general regression test results to account for removal…
bdrodes 11e8139
Crypto: Updated default flows to use taint tracking (this is needed t…
bdrodes 7a57496
Crypto: Missing test update.
bdrodes f524de4
Crypto: Updating insecure iv/nonce to consider if an operation is kno…
bdrodes fdba3ac
Crypto: Fix QL-for-QL alert and auto-format
nicolaswill c6cc4ff
Crypto: Minor fixes to WeakBlockModes, WeakHash to consider SHA3 ok, …
bdrodes 3dedda4
Merge branch 'santander-java-crypto-check' of https://github.com/bdro…
bdrodes deb4373
Crypto: Minor fixes to WeakSymmetricCipher, change to a singular name…
bdrodes fba8087
Crypto: Example query reorg - moving queries of this PR into 'example…
bdrodes 758759a
Crypto: Reused nonce query updates and test updates to address false …
bdrodes 3667365
Crypto: Weak asymmetric key gen size fixes and test.
bdrodes ffd191d
Crypto: missing new endpoint to get the creating operation for a key …
bdrodes d68f3cf
Crypto: InsecureIVorNonceSource now ignored null to avoid being too n…
bdrodes e76ced1
Crypto: Updating weak asymmetric key gen to include key exchange.
bdrodes 08abdb8
Crypto: Adding a "javaConstant" concept to handle config files.
bdrodes 4b241d7
Crypto: adding initial weak hash query overhaul and tests, but no exp…
bdrodes bd068c2
Crypto: Updating expected file for weak asymmetric key gen size.
bdrodes 76128ed
Crypto: Update InsecureIVorNonce to be a path problem.
bdrodes 7847e92
Crypto: Update KDF iteration and count to be path problems
bdrodes 8b5a423
Crypto: Convert ReusedNonce.ql into a path problem.
bdrodes 7e8acd7
Crypto: Update WeakAsymmetricKeyGenSize to a path problem.
bdrodes 55bbcee
Crypto: Make WeakAsymmetricKeyGenSize a path problem.
bdrodes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
22 changes: 22 additions & 0 deletions
22
java/ql/src/experimental/quantum/Analysis/InsecureNonceGeneration.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/** | ||
* @name Insecure nonce at a cipher operation | ||
* @id java/quantum/insecure-nonce | ||
* @description A nonce is generated from a source that is not secure. This can lead to | ||
* vulnerabilities such as replay attacks or key recovery. | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import experimental.quantum.Language | ||
|
||
predicate isInsecureNonceSource(Crypto::NonceArtifactNode n, Crypto::NodeBase src) { | ||
src = n.getSourceNode() and | ||
not src.asElement() instanceof SecureRandomnessInstance | ||
} | ||
|
||
from Crypto::KeyOperationNode op, Crypto::NodeBase src | ||
where isInsecureNonceSource(op.getANonce(), src) | ||
select op, "Operation uses insecure nonce source $@", src, src.toString() |
24 changes: 24 additions & 0 deletions
24
java/ql/src/experimental/quantum/Analysis/NonAESGCMCipher.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* @name Cipher not AES-GCM mode | ||
* @id java/quantum/non-aes-gcm | ||
* @description An AES cipher is in use without GCM | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import experimental.quantum.Language | ||
|
||
class NonAESGCMAlgorithmNode extends Crypto::KeyOperationAlgorithmNode { | ||
NonAESGCMAlgorithmNode() { | ||
this.getAlgorithmType() = Crypto::KeyOpAlg::TSymmetricCipher(Crypto::KeyOpAlg::AES()) and | ||
this.getModeOfOperation().getModeType() != Crypto::KeyOpAlg::GCM() | ||
} | ||
} | ||
|
||
from Crypto::KeyOperationNode op, Crypto::KeyOperationOutputNode codeNode | ||
where op.getAKnownAlgorithm() instanceof NonAESGCMAlgorithmNode and | ||
codeNode = op.getAnOutputArtifact() | ||
select op, "Non-AES-GCM instance." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
java/ql/src/experimental/quantum/Analysis/WeakAsymmetric.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/** | ||
* @name Weak Asymmetric Key Size | ||
* @id java/quantum/weak-asymmetric-key-size | ||
* @description An asymmetric cipher with a short key size is in use | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import java | ||
import experimental.quantum.Language | ||
|
||
from Crypto::KeyOperationAlgorithmNode op, DataFlow::Node configSrc, int keySize, string algName | ||
where | ||
keySize = op.getKeySizeFixed() and | ||
keySize < 2048 and | ||
algName = op.getAlgorithmName() and | ||
// Can't be an elliptic curve | ||
not Crypto::isEllipticCurveAlgorithmName(algName) | ||
select op, | ||
"Use of weak asymmetric key size (" + keySize.toString() + " bits) for algorithm " + | ||
algName.toString() + " at config source $@", configSrc, configSrc.toString() |
31 changes: 31 additions & 0 deletions
31
java/ql/src/experimental/quantum/Analysis/WeakBlockModes.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/** | ||
* @name Weak AES Block mode | ||
* @id java/quantum/weak-block-modes | ||
* @description An AES cipher is in use with an insecure block mode | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import java | ||
import experimental.quantum.Language | ||
|
||
class WeakAESBlockModeAlgNode extends Crypto::KeyOperationAlgorithmNode { | ||
WeakAESBlockModeAlgNode() { | ||
this.getAlgorithmType() = Crypto::KeyOpAlg::TSymmetricCipher(Crypto::KeyOpAlg::AES()) and | ||
( | ||
this.getModeOfOperation().getModeType() = Crypto::KeyOpAlg::ECB() or | ||
this.getModeOfOperation().getModeType() = Crypto::KeyOpAlg::CFB() or | ||
this.getModeOfOperation().getModeType() = Crypto::KeyOpAlg::OFB() or | ||
this.getModeOfOperation().getModeType() = Crypto::KeyOpAlg::CTR() | ||
) | ||
} | ||
} | ||
|
||
from Crypto::KeyOperationNode op, Crypto::KeyOperationOutputNode codeNode | ||
where | ||
op.getAKnownAlgorithm() instanceof WeakAESBlockModeAlgNode and | ||
codeNode = op.getAnOutputArtifact() | ||
select op, "Weak AES block mode instance." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* @name Weak hashes | ||
* @description Finds uses of cryptographic hashing algorithms that are unapproved or otherwise weak. | ||
* @id java/quantum/weak-hashes | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags external/cwe/cwe-327 | ||
* quantum | ||
* experimental | ||
*/ | ||
|
||
import java | ||
import experimental.quantum.Language | ||
|
||
from Crypto::HashAlgorithmNode alg, Crypto::HashType htype, string msg | ||
where | ||
htype = alg.getHashType() and | ||
( | ||
htype != Crypto::SHA2() and | ||
msg = "Use of unapproved hash algorithm or API " + htype.toString() + "." | ||
or | ||
htype = Crypto::SHA2() and | ||
not exists(alg.getDigestLength()) and | ||
msg = | ||
"Use of approved hash algorithm or API type " + htype.toString() + " but unknown digest size." | ||
or | ||
htype = Crypto::SHA2() and | ||
alg.getDigestLength() < 256 and | ||
msg = | ||
"Use of approved hash algorithm or API type " + htype.toString() + " but weak digest size (" + | ||
alg.getDigestLength() + ")." | ||
) | ||
select alg, msg |
20 changes: 20 additions & 0 deletions
20
java/ql/src/experimental/quantum/Analysis/WeakKDFIterationCount.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/** | ||
* @name Weak known key derivation function iteration count | ||
* @description Detects key derivation operations with a known weak iteration count. | ||
* @id java/quantum/weak-kdf-iteration-count | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import java | ||
import experimental.quantum.Language | ||
|
||
from Crypto::KeyDerivationOperationNode op, Literal l | ||
where | ||
op.getIterationCount().asElement() = l and | ||
l.getValue().toInt() < 100000 | ||
select op, "Key derivation operation configures iteration count below 100k: $@", l, | ||
l.getValue().toString() |
20 changes: 20 additions & 0 deletions
20
java/ql/src/experimental/quantum/Analysis/WeakKDFKeySize.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/** | ||
* @name Weak known key derivation function output length | ||
* @description Detects key derivation operations with a known weak output length | ||
* @id java/quantum/weak-kdf-key-size | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import java | ||
import experimental.quantum.Language | ||
|
||
from Crypto::KeyDerivationOperationNode op, Literal l | ||
where | ||
op.getOutputKeySize().asElement() = l and | ||
l.getValue().toInt() < 256 | ||
select op, "Key derivation operation configures output key length below 256: $@", l, | ||
l.getValue().toString() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/** | ||
* @name Cipher is Weak RSA Implementation | ||
* @id java/quantum/weak-rsa | ||
* @description RSA with a key length <2048 found | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags quantum | ||
* experimental | ||
*/ | ||
|
||
import experimental.quantum.Language | ||
|
||
class WeakRsaAlgorithmNode extends Crypto::KeyOperationAlgorithmNode { | ||
WeakRsaAlgorithmNode() { | ||
this.getAlgorithmType() = Crypto::KeyOpAlg::TAsymmetricCipher(Crypto::KeyOpAlg::RSA()) and | ||
this.getKeySizeFixed() < 2048 | ||
} | ||
} | ||
|
||
from Crypto::KeyOperationNode op, string message | ||
where | ||
op.getAKnownAlgorithm() instanceof WeakRsaAlgorithmNode and | ||
message = "Weak RSA instance found with key length <2048" | ||
select op, message |
30 changes: 30 additions & 0 deletions
30
java/ql/src/experimental/quantum/Analysis/WeakSymmetricCiphers.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/** | ||
* @name Weak symmetric ciphers | ||
* @description Finds uses of cryptographic symmetric cipher algorithms that are unapproved or otherwise weak. | ||
* @id java/quantum/weak-ciphers | ||
* @kind problem | ||
* @problem.severity error | ||
* @precision high | ||
* @tags external/cwe/cwe-327 | ||
* quantum | ||
* experimental | ||
*/ | ||
|
||
import java | ||
import experimental.quantum.Language | ||
import Crypto::KeyOpAlg as KeyOpAlg | ||
|
||
from Crypto::KeyOperationAlgorithmNode alg, KeyOpAlg::AlgorithmType algType, string msg | ||
where | ||
algType = alg.getAlgorithmType() and | ||
( | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::DES()) or | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::TRIPLE_DES()) or | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::DOUBLE_DES()) or | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::RC2()) or | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::RC4()) or | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::IDEA()) or | ||
algType = KeyOpAlg::TSymmetricCipher(KeyOpAlg::BLOWFISH()) | ||
) and | ||
msg = "Use of unapproved symmetric cipher algorithm or API: " + algType.toString() + "." | ||
select alg, msg |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable
configSrc
is declared in thefrom
clause but never assigned a value in thewhere
clause, which will cause the query to fail or produce no results. Either removeconfigSrc
from the select statement or add a condition to assign it a value.Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment does indeed point out an issue with the query. I would suggest a rewrite to something like the following (I have not tested this change... it's a draft revision):