hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 02:35:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Use shared CryptographicOperation concept and implement BlockMode getBlockMode()

Browse Source
This commit is contained in:
Alex Ford
2023-02-02 20:28:37 +00:00
parent e5dfbe2c8d
commit 983055b8f9
6 changed files with 121 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll
View File

@@ -387,7 +387,7 @@ private class CryptographicOperationFlowCharacteristic extends NotASinkCharacter
CryptographicOperationFlowCharacteristic() { this = "CryptographicOperationFlow" }
override predicate appliesToEndpoint(DataFlow::Node n) {
any(CryptographicOperation op).getInput() = n
any(CryptographicOperation op).getAnInput() = n
}
}

7
javascript/ql/lib/semmle/javascript/Concepts.qll
View File

@@ -110,3 +110,10 @@ abstract class PersistentWriteAccess extends DataFlow::Node {
*/
abstract DataFlow::Node getValue();
}
/**
* Provides models for cryptographic things.
*/
module Cryptography {
import semmle.javascript.internal.ConceptsShared::Cryptography
}

158
javascript/ql/lib/semmle/javascript/frameworks/CryptoLibraries.qll
View File

@@ -3,22 +3,7 @@
*/
import javascript
import semmle.javascript.security.CryptoAlgorithms
/**
* An application of a cryptographic algorithm.
*/
abstract class CryptographicOperation extends DataFlow::Node {
/**
* Gets the input the algorithm is used on, e.g. the plain text input to be encrypted.
*/
abstract DataFlow::Node getInput();
/**
* Gets the applied algorithm.
*/
abstract CryptographicAlgorithm getAlgorithm();
}
import semmle.javascript.Concepts::Cryptography
/**
* A key used in a cryptographic algorithm.
@@ -52,13 +37,20 @@ class CryptographicKeyCredentialsExpr extends CredentialsNode instanceof Cryptog
override string getCredentialsKind() { result = "key" }
}
// Holds if `algorithm` is an `EncryptionAlgorithm` that uses a block cipher
private predicate isBlockEncryptionAlgorithm(CryptographicAlgorithm algorithm) {
algorithm instanceof EncryptionAlgorithm and
not algorithm.(EncryptionAlgorithm).isStreamCipher()
}
/**
* A model of the asmCrypto library.
*/
private module AsmCrypto {
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm; // non-functional
private string algorithmName;
Apply() {
/*
@@ -71,17 +63,22 @@ private module AsmCrypto {
* ```
*/
exists(DataFlow::SourceNode asmCrypto, string algorithmName |
exists(DataFlow::SourceNode asmCrypto |
asmCrypto = DataFlow::globalVarRef("asmCrypto") and
algorithm.matchesName(algorithmName) and
this = asmCrypto.getAPropertyRead(algorithmName).getAMemberCall(_) and
input = this.getAnArgument()
input = this.getArgument(0)
)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
override BlockMode getBlockMode() {
isBlockEncryptionAlgorithm(this.getAlgorithm()) and
result.matchesString(algorithmName)
}
}
}
@@ -93,7 +90,7 @@ private module BrowserIdCrypto {
Key() { this = any(Apply apply).getKey() }
}
private class Apply extends CryptographicOperation instanceof DataFlow::MethodCallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::MethodCallNode {
CryptographicAlgorithm algorithm; // non-functional
Apply() {
@@ -126,10 +123,13 @@ private module BrowserIdCrypto {
)
}
override DataFlow::Node getInput() { result = super.getArgument(0) }
override DataFlow::Node getAnInput() { result = super.getArgument(0) }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// not relevant for browserid-crypto
override BlockMode getBlockMode() { none() }
DataFlow::Node getKey() { result = super.getArgument(1) }
}
}
@@ -140,6 +140,7 @@ private module BrowserIdCrypto {
private module NodeJSCrypto {
private class InstantiatedAlgorithm extends DataFlow::CallNode {
CryptographicAlgorithm algorithm; // non-functional
private string algorithmName;
InstantiatedAlgorithm() {
/*
@@ -158,11 +159,25 @@ private module NodeJSCrypto {
exists(DataFlow::SourceNode mod |
mod = DataFlow::moduleImport("crypto") and
this = mod.getAMemberCall("create" + ["Hash", "Hmac", "Sign", "Cipher"]) and
algorithm.matchesName(this.getArgument(0).getStringValue())
algorithmName = this.getArgument(0).getStringValue() and
algorithm.matchesName(algorithmName)
)
}
CryptographicAlgorithm getAlgorithm() { result = algorithm }
private BlockMode getExplicitBlockMode() { result.matchesString(algorithmName) }
BlockMode getBlockMode() {
isBlockEncryptionAlgorithm(this.getAlgorithm()) and
(
if exists(this.getExplicitBlockMode())
then result = this.getExplicitBlockMode()
else
// CBC is the default if not explicitly specified
result = "CBC"
)
}
}
private class CreateKey extends CryptographicKeyCreation, DataFlow::CallNode {
@@ -211,14 +226,16 @@ private module NodeJSCrypto {
override predicate isSymmetricKey() { none() }
}
private class Apply extends CryptographicOperation instanceof DataFlow::MethodCallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::MethodCallNode {
InstantiatedAlgorithm instantiation;
Apply() { this = instantiation.getAMethodCall(any(string m | m = "update" or m = "write")) }
override DataFlow::Node getInput() { result = super.getArgument(0) }
override DataFlow::Node getAnInput() { result = super.getArgument(0) }
override CryptographicAlgorithm getAlgorithm() { result = instantiation.getAlgorithm() }
override BlockMode getBlockMode() { result = instantiation.getBlockMode() }
}
private class Key extends CryptographicKey {
@@ -307,7 +324,7 @@ private module CryptoJS {
input = result.getArgument(0)
}
private class Apply extends CryptographicOperation {
private class Apply extends CryptographicOperation::Range, DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm; // non-functional
@@ -316,9 +333,31 @@ private module CryptoJS {
this = getDirectApplication(input, algorithm)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// e.g. CryptoJS.AES.encrypt("msg", "key", { mode: CryptoJS.mode.<modeString> })
private BlockMode getExplicitBlockMode() {
exists(DataFlow::ObjectLiteralNode o, DataFlow::SourceNode modeNode, string modeString |
modeNode = API::moduleImport("crypto-js").getMember("mode").getMember(modeString).asSource() and
o.flowsTo(this.getArgument(2)) and
modeNode = o.getAPropertySource("mode")
|
result.matchesString(modeString)
)
}
override BlockMode getBlockMode() {
isBlockEncryptionAlgorithm(this.getAlgorithm()) and
(
if exists(this.getExplicitBlockMode())
then result = this.getExplicitBlockMode()
else
// CBC is the default if not explicitly specified
result = "CBC"
)
}
}
private class Key extends CryptographicKey {
@@ -374,7 +413,7 @@ private module CryptoJS {
* A model of the TweetNaCl library.
*/
private module TweetNaCl {
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm;
@@ -401,9 +440,12 @@ private module TweetNaCl {
)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// No block ciphers implemented
override BlockMode getBlockMode() { none() }
}
}
@@ -434,7 +476,7 @@ private module HashJs {
)
}
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm; // non-functional
@@ -456,9 +498,12 @@ private module HashJs {
input = super.getArgument(0)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// not relevant for hash.js
override BlockMode getBlockMode() { none() }
}
}
@@ -478,19 +523,20 @@ private module Forge {
private class KeyCipher extends Cipher {
DataFlow::Node key;
CryptographicAlgorithm algorithm; // non-functional
private string blockModeString;
KeyCipher() {
exists(DataFlow::SourceNode mod, string algorithmName |
mod = getAnImportNode() and
algorithm.matchesName(algorithmName)
|
exists(string createName, string cipherName, string cipherPrefix, string cipherSuffix |
exists(string createName, string cipherName, string cipherPrefix |
// `require('forge').cipher.createCipher("3DES-CBC").update("secret", "key");`
(createName = "createCipher" or createName = "createDecipher") and
this = mod.getAPropertyRead("cipher").getAMemberCall(createName) and
this.getArgument(0).mayHaveStringValue(cipherName) and
cipherName = cipherPrefix + "-" + cipherSuffix and
cipherSuffix = ["CBC", "CFB", "CTR", "ECB", "GCM", "OFB"] and
cipherName = cipherPrefix + "-" + blockModeString and
blockModeString = ["CBC", "CFB", "CTR", "ECB", "GCM", "OFB"] and
algorithmName = cipherPrefix and
key = this.getArgument(1)
)
@@ -500,7 +546,8 @@ private module Forge {
createName = "createEncryptionCipher" or createName = "createDecryptionCipher"
|
this = mod.getAPropertyRead(algorithmName).getAMemberCall(createName) and
key = this.getArgument(0)
key = this.getArgument(0) and
blockModeString = algorithmName
)
)
}
@@ -508,6 +555,11 @@ private module Forge {
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
DataFlow::Node getKey() { result = key }
BlockMode getBlockMode() {
isBlockEncryptionAlgorithm(this.getAlgorithm()) and
result.matchesString(blockModeString)
}
}
private class NonKeyCipher extends Cipher {
@@ -527,21 +579,22 @@ private module Forge {
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
}
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm; // non-functional
private Cipher cipher;
Apply() {
exists(Cipher cipher |
this = cipher.getAMemberCall("update") and
super.getArgument(0) = input and
algorithm = cipher.getAlgorithm()
)
this = cipher.getAMemberCall("update") and
super.getArgument(0) = input and
algorithm = cipher.getAlgorithm()
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
override BlockMode getBlockMode() { result = cipher.(KeyCipher).getBlockMode() }
}
private class Key extends CryptographicKey {
@@ -586,7 +639,7 @@ private module Forge {
* A model of the md5 library.
*/
private module Md5 {
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm;
@@ -600,9 +653,12 @@ private module Md5 {
)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// not relevant for md5
override BlockMode getBlockMode() { none() }
}
}
@@ -610,7 +666,7 @@ private module Md5 {
* A model of the bcrypt, bcryptjs, bcrypt-nodejs libraries.
*/
private module Bcrypt {
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm;
@@ -633,9 +689,12 @@ private module Bcrypt {
)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// not relevant for bcrypt
override BlockMode getBlockMode() { none() }
}
}
@@ -643,7 +702,7 @@ private module Bcrypt {
* A model of the hasha library.
*/
private module Hasha {
private class Apply extends CryptographicOperation instanceof DataFlow::CallNode {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm;
@@ -659,9 +718,12 @@ private module Hasha {
)
}
override DataFlow::Node getInput() { result = input }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
// not relevant for hasha
override BlockMode getBlockMode() { none() }
}
}

2
javascript/ql/lib/semmle/javascript/security/dataflow/BrokenCryptoAlgorithmCustomizations.qll
View File

@@ -41,7 +41,7 @@ module BrokenCryptoAlgorithm {
WeakCryptographicOperationSink() {
exists(CryptographicOperation application |
application.getAlgorithm().isWeak() and
this = application.getInput()
this = application.getAnInput()
)
}
}

2
javascript/ql/lib/semmle/javascript/security/dataflow/InsufficientPasswordHashCustomizations.qll
View File

@@ -47,7 +47,7 @@ module InsufficientPasswordHash {
application.getAlgorithm().isWeak() or
not application.getAlgorithm() instanceof PasswordHashingAlgorithm
|
this = application.getInput()
this = application.getAnInput()
)
}
}

2
javascript/ql/test/library-tests/CryptoLibraries/CryptographicOperation.ql
View File

@@ -1,4 +1,4 @@
import javascript
from CryptographicOperation operation
select operation, operation.getAlgorithm().getName(), operation.getInput()
select operation, operation.getAlgorithm().getName(), operation.getAnInput()