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

Port changes to JavaScript.

Browse Source
This commit is contained in:
Max Schaefer
2023-10-26 14:47:24 +01:00
parent 3939167ba2
commit 741735cc83
5 changed files with 83 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -51,6 +51,7 @@ private module AsmCrypto {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm; // non-functional
DataFlow::PropRead algorithmSelection;
private string algorithmName;
private string methodName;
@@ -68,11 +69,14 @@ private module AsmCrypto {
exists(DataFlow::SourceNode asmCrypto |
asmCrypto = DataFlow::globalVarRef("asmCrypto") and
algorithm.matchesName(algorithmName) and
this = asmCrypto.getAPropertyRead(algorithmName).getAMemberCall(methodName) and
algorithmSelection = asmCrypto.getAPropertyRead(algorithmName) and
this = algorithmSelection.getAMemberCall(methodName) and
input = this.getArgument(0)
)
}
override DataFlow::Node getInitialization() { result = algorithmSelection }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -103,6 +107,7 @@ private module BrowserIdCrypto {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::MethodCallNode {
CryptographicAlgorithm algorithm; // non-functional
DataFlow::CallNode keygen;
Apply() {
/*
@@ -122,8 +127,7 @@ private module BrowserIdCrypto {
*/
exists(
DataFlow::SourceNode mod, DataFlow::Node algorithmNameNode, DataFlow::CallNode keygen,
DataFlow::FunctionNode callback
DataFlow::SourceNode mod, DataFlow::Node algorithmNameNode, DataFlow::FunctionNode callback
|
mod = DataFlow::moduleImport("browserid-crypto") and
keygen = mod.getAMemberCall("generateKeypair") and
@@ -134,6 +138,8 @@ private module BrowserIdCrypto {
)
}
override DataFlow::Node getInitialization() { result = keygen }
override DataFlow::Node getAnInput() { result = super.getArgument(0) }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -239,6 +245,8 @@ private module NodeJSCrypto {
Apply() { this = instantiation.getAMethodCall(any(string m | m = "update" or m = "write")) }
override DataFlow::Node getInitialization() { result = instantiation }
override DataFlow::Node getAnInput() { result = super.getArgument(0) }
override CryptographicAlgorithm getAlgorithm() { result = instantiation.getAlgorithm() }
@@ -324,7 +332,9 @@ private module CryptoJS {
)
}
private API::CallNode getEncryptionApplication(API::Node input, CryptographicAlgorithm algorithm) {
private API::CallNode getEncryptionApplication(
API::Node input, API::Node algorithmNode, CryptographicAlgorithm algorithm
) {
/*
* ```
* var CryptoJS = require("crypto-js");
@@ -338,11 +348,12 @@ private module CryptoJS {
* Also matches where `CryptoJS.<algorithmName>` has been replaced by `require("crypto-js/<algorithmName>")`
*/
result = getAlgorithmNode(algorithm).getMember("encrypt").getACall() and
algorithmNode = getAlgorithmNode(algorithm) and
result = algorithmNode.getMember("encrypt").getACall() and
input = result.getParameter(0)
}
private API::CallNode getDirectApplication(API::Node input, CryptographicAlgorithm algorithm) {
private API::CallNode getDirectApplication(API::Node input, API::Node algorithmNode, CryptographicAlgorithm algorithm) {
/*
* ```
* var CryptoJS = require("crypto-js");
@@ -356,8 +367,8 @@ private module CryptoJS {
* An `Hmac`-prefix of <algorithmName> is ignored.
* Also matches where `CryptoJS.<algorithmName>` has been replaced by `require("crypto-js/<algorithmName>")`
*/
result = getAlgorithmNode(algorithm).getACall() and
algorithmNode = getAlgorithmNode(algorithm) and
result = algorithmNode.getACall() and
input = result.getParameter(0)
}
@@ -389,18 +400,23 @@ private module CryptoJS {
private class Apply extends CryptographicOperation::Range instanceof API::CallNode {
API::Node input;
CryptographicAlgorithm algorithm; // non-functional
DataFlow::Node instantiation;
Apply() {
this = getEncryptionApplication(input, algorithm)
or
this = getDirectApplication(input, algorithm)
or
exists(InstantiatedAlgorithm instantiation |
this = getUpdatedApplication(input, instantiation) and
algorithm = instantiation.getAlgorithm()
exists(API::Node algorithmNode |
this = getEncryptionApplication(input, algorithmNode, algorithm)
or
this = getDirectApplication(input, algorithmNode, algorithm)
|
instantiation = algorithmNode.asSource()
)
or
this = getUpdatedApplication(input, instantiation) and
algorithm = instantiation.(InstantiatedAlgorithm).getAlgorithm()
}
override DataFlow::Node getInitialization() { result = instantiation }
override DataFlow::Node getAnInput() { result = input.asSink() }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -504,6 +520,8 @@ private module TweetNaCl {
)
}
override DataFlow::Node getInitialization() { result = this }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -539,6 +557,7 @@ private module HashJs {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm; // non-functional
DataFlow::CallNode init;
Apply() {
/*
@@ -553,11 +572,13 @@ private module HashJs {
* ```
* Also matches where `hash.<algorithmName>()` has been replaced by a more specific require a la `require("hash.js/lib/hash/sha/512")`
*/
this = getAlgorithmNode(algorithm).getAMemberCall("update") and
init = getAlgorithmNode(algorithm) and
this = init.getAMemberCall("update") and
input = super.getArgument(0)
}
override DataFlow::Node getInitialization() { result = init }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -653,6 +674,8 @@ private module Forge {
algorithm = cipher.getAlgorithm()
}
override DataFlow::Node getInitialization() { result = cipher }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -715,6 +738,8 @@ private module Md5 {
super.getArgument(0) = input
}
override DataFlow::Node getInitialization() { result = this }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -731,17 +756,21 @@ private module Bcrypt {
private class Apply extends CryptographicOperation::Range instanceof DataFlow::CallNode {
DataFlow::Node input;
CryptographicAlgorithm algorithm;
API::Node init;
Apply() {
// `require("bcrypt").hash(password);` with minor naming variations
algorithm.matchesName("BCRYPT") and
init = API::moduleImport(["bcrypt", "bcryptjs", "bcrypt-nodejs"]) and
this =
API::moduleImport(["bcrypt", "bcryptjs", "bcrypt-nodejs"])
init
.getMember(["hash", "hashSync"])
.getACall() and
super.getArgument(0) = input
}
override DataFlow::Node getInitialization() { result = init.asSource() }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }
@@ -769,6 +798,8 @@ private module Hasha {
)
}
override DataFlow::Node getInitialization() { result = this }
override DataFlow::Node getAnInput() { result = input }
override CryptographicAlgorithm getAlgorithm() { result = algorithm }

6
javascript/ql/lib/semmle/javascript/internal/ConceptsShared.qll
View File

@@ -40,6 +40,9 @@ module Cryptography {
/** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
CryptographicAlgorithm getAlgorithm() { result = super.getAlgorithm() }
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
DataFlow::Node getInitialization() { result = super.getInitialization() }
/** Gets an input the algorithm is used on, for example the plain text input to be encrypted. */
DataFlow::Node getAnInput() { result = super.getAnInput() }
@@ -65,6 +68,9 @@ module Cryptography {
* extend `CryptographicOperation` instead.
*/
abstract class Range extends DataFlow::Node {
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
abstract DataFlow::Node getInitialization();
/** Gets the algorithm used, if it matches a known `CryptographicAlgorithm`. */
abstract CryptographicAlgorithm getAlgorithm();

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

@@ -19,7 +19,10 @@ module BrokenCryptoAlgorithm {
/**
* A data flow sink for sensitive information in broken or weak cryptographic algorithms.
*/
abstract class Sink extends DataFlow::Node { }
abstract class Sink extends DataFlow::Node {
/** Gets the data-flow node where the cryptographic algorithm used in this operation is configured. */
abstract DataFlow::Node getInitialization();
}
/**
* A sanitizer for sensitive information in broken or weak cryptographic algorithms.
@@ -38,15 +41,17 @@ module BrokenCryptoAlgorithm {
* An expression used by a broken or weak cryptographic algorithm.
*/
class WeakCryptographicOperationSink extends Sink {
CryptographicOperation application;
WeakCryptographicOperationSink() {
exists(CryptographicOperation application |
(
application.getAlgorithm().isWeak()
or
application.getBlockMode().isWeak()
) and
this = application.getAnInput()
)
(
application.getAlgorithm().isWeak()
or
application.getBlockMode().isWeak()
) and
this = application.getAnInput()
}
override DataFlow::Node getInitialization() { result = application.getInitialization() }
}
}

13
javascript/ql/src/Security/CWE-327/BrokenCryptoAlgorithm.ql
View File

@@ -16,9 +16,14 @@ import semmle.javascript.security.dataflow.BrokenCryptoAlgorithmQuery
import semmle.javascript.security.SensitiveActions
import DataFlow::PathGraph
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
from
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, Source sourceNode,
Sink sinkNode
where
cfg.hasFlowPath(source, sink) and
not source.getNode() instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
select sink.getNode(), source, sink, "A broken or weak cryptographic algorithm depends on $@.",
source.getNode(), "sensitive data from " + source.getNode().(Source).describe()
sourceNode = source.getNode() and
sinkNode = sink.getNode() and
not sourceNode instanceof CleartextPasswordExpr // flagged by js/insufficient-password-hash
select sinkNode, source, sink,
"A broken or weak cryptographic algorithm (configured $@) depends on $@.",
sinkNode.getInitialization(), "here", sourceNode, "sensitive data from " + sourceNode.describe()

10
javascript/ql/test/query-tests/Security/CWE-327/BrokenCryptoAlgorithm.expected
View File

@@ -26,8 +26,8 @@ edges
| tst.js:19:17:19:24 | password | tst.js:19:17:19:24 | password |
| tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText |
#select
| tst.js:11:17:11:26 | secretText | tst.js:3:18:3:24 | trusted | tst.js:11:17:11:26 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
| tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:11:17:11:26 | secretText | sensitive data from an access to secretText |
| tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | A broken or weak cryptographic algorithm depends on $@. | tst.js:17:17:17:25 | o.trusted | sensitive data from an access to trusted |
| tst.js:22:21:22:30 | secretText | tst.js:3:18:3:24 | trusted | tst.js:22:21:22:30 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
| tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | A broken or weak cryptographic algorithm depends on $@. | tst.js:22:21:22:30 | secretText | sensitive data from an access to secretText |
| tst.js:11:17:11:26 | secretText | tst.js:3:18:3:24 | trusted | tst.js:11:17:11:26 | secretText | A broken or weak cryptographic algorithm (configured $@) depends on $@. | tst.js:5:19:5:49 | crypto. ... ', key) | here | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
| tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | tst.js:11:17:11:26 | secretText | A broken or weak cryptographic algorithm (configured $@) depends on $@. | tst.js:5:19:5:49 | crypto. ... ', key) | here | tst.js:11:17:11:26 | secretText | sensitive data from an access to secretText |
| tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | tst.js:17:17:17:25 | o.trusted | A broken or weak cryptographic algorithm (configured $@) depends on $@. | tst.js:5:19:5:49 | crypto. ... ', key) | here | tst.js:17:17:17:25 | o.trusted | sensitive data from an access to trusted |
| tst.js:22:21:22:30 | secretText | tst.js:3:18:3:24 | trusted | tst.js:22:21:22:30 | secretText | A broken or weak cryptographic algorithm (configured $@) depends on $@. | tst.js:21:22:21:60 | crypto. ... ', key) | here | tst.js:3:18:3:24 | trusted | sensitive data from an access to trusted |
| tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | tst.js:22:21:22:30 | secretText | A broken or weak cryptographic algorithm (configured $@) depends on $@. | tst.js:21:22:21:60 | crypto. ... ', key) | here | tst.js:22:21:22:30 | secretText | sensitive data from an access to secretText |