diff --git a/change-notes/1.20/analysis-javascript.md b/change-notes/1.20/analysis-javascript.md index f6db256578d0..6db9e5ea3d52 100644 --- a/change-notes/1.20/analysis-javascript.md +++ b/change-notes/1.20/analysis-javascript.md @@ -20,6 +20,7 @@ | **Query** | **Expected impact** | **Change** | |--------------------------------------------|------------------------------|------------------------------------------------------------------------------| | Client-side cross-site scripting | More results | This rule now recognizes WinJS functions that are vulnerable to HTML injection. | +| Insecure randomness | More results | This rule now flags insecure uses of `crypto.pseudoRandomBytes`. | | Unused parameter | Fewer false-positive results | This rule no longer flags parameters with leading underscore. | | Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements, and no longer flags variables with leading underscore. | diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll b/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll index bfcdde9b00bd..aed4d712e464 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/InsecureRandomness.qll @@ -68,7 +68,7 @@ module InsecureRandomness { * A simple random number generator that is not cryptographically secure. */ class DefaultSource extends Source, DataFlow::ValueNode { - override CallExpr astNode; + override InvokeExpr astNode; DefaultSource() { exists(DataFlow::ModuleImportNode mod, string name | mod.getPath() = name | @@ -98,6 +98,9 @@ module InsecureRandomness { or // (new require('chance')).() this = DataFlow::moduleImport("chance").getAnInstantiation().getAMemberInvocation(_) + or + // require('crypto').pseudoRandomBytes() + this = DataFlow::moduleMember("crypto", "pseudoRandomBytes").getAnInvocation() } } diff --git a/javascript/ql/test/library-tests/Security/CWE-338/InsecureRandomnessSource.expected b/javascript/ql/test/library-tests/Security/CWE-338/InsecureRandomnessSource.expected index e51ede139b07..0f4718ebc865 100644 --- a/javascript/ql/test/library-tests/Security/CWE-338/InsecureRandomnessSource.expected +++ b/javascript/ql/test/library-tests/Security/CWE-338/InsecureRandomnessSource.expected @@ -6,3 +6,5 @@ | tst.js:15:1:15:12 | randomSeed() | | tst.js:18:1:18:14 | uniqueRandom() | | tst.js:22:1:22:12 | chance.XYZ() | +| tst.js:25:1:25:29 | crypto. ... es(100) | +| tst.js:26:1:26:33 | new cry ... es(100) | diff --git a/javascript/ql/test/library-tests/Security/CWE-338/tst.js b/javascript/ql/test/library-tests/Security/CWE-338/tst.js index 9879d996401b..ac5adfa01ef0 100644 --- a/javascript/ql/test/library-tests/Security/CWE-338/tst.js +++ b/javascript/ql/test/library-tests/Security/CWE-338/tst.js @@ -20,3 +20,7 @@ uniqueRandom(); var Chance = require('chance'), chance = new Chance(); chance.XYZ(); + +let crypto = require('crypto'); +crypto.pseudoRandomBytes(100); +new crypto.pseudoRandomBytes(100);