diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp new file mode 100644 index 000000000000..16196386d12a --- /dev/null +++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.qhelp @@ -0,0 +1,37 @@ + + + + +

+Placeholder +

+
+ + +

+Placeholder +

+
+ + +

+Placeholder +

+ +
+ + +
  • +OWASP: +Code Injection. +
  • +
  • +MDN: Global functions. +
  • +
  • +MDN: Function constructor. +
  • +
    +
    diff --git a/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql new file mode 100644 index 000000000000..83695656aa59 --- /dev/null +++ b/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql @@ -0,0 +1,66 @@ +/** + * @name Improper code sanitization + * @description Escaping code as HTML does not provide protection against code-injection. + * @kind path-problem + * @problem.severity error + * @precision high + * @id js/bad-code-sanitization + * @tags security + * external/cwe/cwe-094 + * external/cwe/cwe-079 + * external/cwe/cwe-116 + */ + +import javascript +import semmle.javascript.security.dataflow.ImproperCodeSanitization::ImproperCodeSanitization +import DataFlow::PathGraph +private import semmle.javascript.heuristics.HeuristicSinks +private import semmle.javascript.security.dataflow.CodeInjectionCustomizations + +/** + * Gets a type-tracked instance of `RemoteFlowSource` using type-tracker `t`. + */ +private DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) { + t.start() and + result instanceof RemoteFlowSource + or + exists(DataFlow::TypeTracker t2 | result = remoteFlow(t2).track(t2, t)) +} + +/** + * Gets a type-tracked reference to a `RemoteFlowSource`. + */ +private DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) } + +/** + * Gets a type-back-tracked instance of a code-injection sink using type-tracker `t`. + */ +private DataFlow::Node endsInCodeInjectionSink(DataFlow::TypeBackTracker t) { + t.start() and + ( + result instanceof CodeInjection::Sink + or + result instanceof HeuristicCodeInjectionSink and + not result instanceof StringOps::ConcatenationRoot // the heuristic CodeInjection sink looks for string-concats, we are not interrested in those here. + ) + or + exists(DataFlow::TypeBackTracker t2 | t = t2.smallstep(result, endsInCodeInjectionSink(t2))) +} + +/** + * Gets a reference to to a data-flow node that ends in a code-injection sink. + */ +private DataFlow::Node endsInCodeInjectionSink() { + result = endsInCodeInjectionSink(DataFlow::TypeBackTracker::end()) +} + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where + cfg.hasFlowPath(source, sink) and + // Basic detection of duplicate results with `js/code-injection`. + not ( + sink.getNode().(StringOps::ConcatenationLeaf).getRoot() = endsInCodeInjectionSink() and + remoteFlow().flowsTo(source.getNode().(DataFlow::InvokeNode).getAnArgument()) + ) +select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.", + source.getNode(), "Improperly sanitized value" diff --git a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll b/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll index 64cfff5dd756..d1228c84248d 100644 --- a/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll +++ b/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll @@ -17,32 +17,12 @@ private import semmle.javascript.security.dataflow.RegExpInjectionCustomizations private import semmle.javascript.security.dataflow.ClientSideUrlRedirectCustomizations private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations private import semmle.javascript.security.dataflow.InsecureRandomnessCustomizations +private import HeuristicSinks as Sinks -/** - * A heuristic sink for data flow in a security query. - */ -abstract class HeuristicSink extends DataFlow::Node { } +class HeuristicSink = Sinks::HeuristicSink; -private class HeuristicCodeInjectionSink extends HeuristicSink, CodeInjection::Sink { - HeuristicCodeInjectionSink() { - isAssignedTo(this, "$where") - or - isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)") - or - isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec` - or - exists(string srcPattern | - // function/lambda syntax anywhere - srcPattern = "(?s).*function\\s*\\(.*\\).*" or - srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*" - | - isConcatenatedWithString(this, srcPattern) - ) - or - // dynamic property name - isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*") - } -} +private class HeuristicCodeInjectionSink extends Sinks::HeuristicCodeInjectionSink, + CodeInjection::Sink { } private class HeuristicCommandInjectionSink extends HeuristicSink, CommandInjection::Sink { HeuristicCommandInjectionSink() { diff --git a/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll b/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll new file mode 100644 index 000000000000..ae26e66b3b4f --- /dev/null +++ b/javascript/ql/src/semmle/javascript/heuristics/HeuristicSinks.qll @@ -0,0 +1,35 @@ +/** + * Provides heuristically recognized sinks for security queries. + */ + +import javascript +private import SyntacticHeuristics + +/** + * A heuristic sink for data flow in a security query. + */ +abstract class HeuristicSink extends DataFlow::Node { } + +/** + * A heuristically recognized sink for `js/code-injection` vulnerabilities. + */ +class HeuristicCodeInjectionSink extends HeuristicSink { + HeuristicCodeInjectionSink() { + isAssignedTo(this, "$where") + or + isAssignedToOrConcatenatedWith(this, "(?i)(command|cmd|exec|code|script|program)") + or + isArgTo(this, "(?i)(eval|run)") // "exec" clashes too often with `RegExp.prototype.exec` + or + exists(string srcPattern | + // function/lambda syntax anywhere + srcPattern = "(?s).*function\\s*\\(.*\\).*" or + srcPattern = "(?s).*(\\(.*\\)|[A-Za-z_]+)\\s?=>.*" + | + isConcatenatedWithString(this, srcPattern) + ) + or + // dynamic property name + isConcatenatedWithStrings("(?is)[a-z]+\\[", this, "(?s)\\].*") + } +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll new file mode 100644 index 000000000000..ebe7c1b834fa --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitization.qll @@ -0,0 +1,30 @@ +/** + * Provides a taint-tracking configuration for reasoning about improper code + * sanitization. + * + * Note, for performance reasons: only import this file if + * `ImproperCodeSanitization::Configuration` is needed, otherwise + * `ImproperCodeSanitizationCustomizations` should be imported instead. + */ + +import javascript + +/** + * Classes and predicates for reasoning about improper code sanitization. + */ +module ImproperCodeSanitization { + import ImproperCodeSanitizationCustomizations::ImproperCodeSanitization + + /** + * A taint-tracking configuration for reasoning about improper code sanitization vulnerabilities. + */ + class Configuration extends TaintTracking::Configuration { + Configuration() { this = "ImproperCodeSanitization" } + + override predicate isSource(DataFlow::Node source) { source instanceof Source } + + override predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + override predicate isSanitizer(DataFlow::Node sanitizer) { sanitizer instanceof Sanitizer } + } +} diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll new file mode 100644 index 000000000000..826797050209 --- /dev/null +++ b/javascript/ql/src/semmle/javascript/security/dataflow/ImproperCodeSanitizationCustomizations.qll @@ -0,0 +1,68 @@ +/** + * Provides default sources, sinks and sanitizers for reasoning about + * improper code sanitization, as well as extension points for + * adding your own. + */ + +import javascript + +/** + * Classes and predicates for reasoning about improper code sanitization. + */ +module ImproperCodeSanitization { + /** + * A data flow source for improper code sanitization. + */ + abstract class Source extends DataFlow::Node { } + + /** + * A data flow sink for improper code sanitization. + */ + abstract class Sink extends DataFlow::Node { } + + /** + * A sanitizer for improper code sanitization. + */ + abstract class Sanitizer extends DataFlow::Node { } + + /** + * A call to a HTML sanitizer seen as a source for improper code sanitization + */ + class HtmlSanitizerCallAsSource extends Source { + HtmlSanitizerCallAsSource() { this instanceof HtmlSanitizerCall } + } + + /** + * A call to `JSON.stringify()` seen as a source for improper code sanitization + */ + class JSONStringifyAsSource extends Source { + JSONStringifyAsSource() { this = DataFlow::globalVarRef("JSON").getAMemberCall("stringify") } + } + + /** + * A leaf in a string-concatenation, where the string-concatenation constructs code that looks like a function. + */ + class FunctionStringConstruction extends Sink, StringOps::ConcatenationLeaf { + FunctionStringConstruction() { + exists(StringOps::ConcatenationRoot root, int i | + root.getOperand(i) = this and + not exists(this.getStringValue()) + | + exists(StringOps::ConcatenationLeaf functionLeaf | + functionLeaf = root.getOperand(any(int j | j < i)) + | + functionLeaf + .getStringValue() + .regexpMatch([".*function( )?([a-zA-Z0-9]+)?( )?\\(.*", ".*eval\\(.*", + ".*new Function\\(.*", "(^|.*[^a-zA-Z0-9])\\(.*\\)( )?=>.*"]) + ) + ) + } + } + + /** + * A call to `String.prototype.replace` seen as a sanitizer for improper code sanitization. + * All calls to replace that happens after the initial improper sanitization is seen as a sanitizer. + */ + class StringReplaceCallAsSanitizer extends Sanitizer, StringReplaceCall { } +} diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected index 6182246e9297..7151a14f63e7 100644 --- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected @@ -64,6 +64,11 @@ nodes | angularjs.js:53:32:53:39 | location | | angularjs.js:53:32:53:46 | location.search | | angularjs.js:53:32:53:46 | location.search | +| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | | express.js:7:24:7:69 | "return ... + "];" | | express.js:7:24:7:69 | "return ... + "];" | | express.js:7:44:7:62 | req.param("wobble") | @@ -193,6 +198,10 @@ edges | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | @@ -261,6 +270,7 @@ edges | angularjs.js:47:16:47:30 | location.search | angularjs.js:47:16:47:23 | location | angularjs.js:47:16:47:30 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:47:16:47:23 | location | User-provided value | | angularjs.js:50:22:50:36 | location.search | angularjs.js:50:22:50:29 | location | angularjs.js:50:22:50:36 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:50:22:50:29 | location | User-provided value | | angularjs.js:53:32:53:46 | location.search | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | $@ flows to here and is interpreted as code. | angularjs.js:53:32:53:39 | location | User-provided value | +| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | $@ flows to here and is interpreted as code. | bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | User-provided value | | express.js:7:24:7:69 | "return ... + "];" | express.js:7:44:7:62 | req.param("wobble") | express.js:7:24:7:69 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:7:44:7:62 | req.param("wobble") | User-provided value | | express.js:9:34:9:79 | "return ... + "];" | express.js:9:54:9:72 | req.param("wobble") | express.js:9:34:9:79 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:9:54:9:72 | req.param("wobble") | User-provided value | | express.js:12:8:12:53 | "return ... + "];" | express.js:12:28:12:46 | req.param("wobble") | express.js:12:8:12:53 | "return ... + "];" | $@ flows to here and is interpreted as code. | express.js:12:28:12:46 | req.param("wobble") | User-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected index a4b7a3b1ea98..cb6966f4f115 100644 --- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected @@ -64,6 +64,11 @@ nodes | angularjs.js:53:32:53:39 | location | | angularjs.js:53:32:53:46 | location.search | | angularjs.js:53:32:53:46 | location.search | +| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | | eslint-escope-build.js:20:22:20:22 | c | | eslint-escope-build.js:20:22:20:22 | c | | eslint-escope-build.js:21:16:21:16 | c | @@ -197,6 +202,10 @@ edges | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | | angularjs.js:53:32:53:39 | location | angularjs.js:53:32:53:46 | location.search | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:14:54:67 | `(funct ... "))}))` | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:44:54:62 | req.param("wobble") | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c | | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c | | eslint-escope-build.js:20:22:20:22 | c | eslint-escope-build.js:21:16:21:16 | c | diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected new file mode 100644 index 000000000000..79c97be21a71 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected @@ -0,0 +1,59 @@ +nodes +| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | +| bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | +| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | +| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | +| bad-code-sanitization.js:6:11:6:25 | statements | +| bad-code-sanitization.js:6:24:6:25 | [] | +| bad-code-sanitization.js:7:21:7:70 | `${name ... key])}` | +| bad-code-sanitization.js:7:31:7:43 | safeProp(key) | +| bad-code-sanitization.js:8:27:8:36 | statements | +| bad-code-sanitization.js:8:27:8:46 | statements.join(';') | +| bad-code-sanitization.js:8:27:8:46 | statements.join(';') | +| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | +| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | +| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | +| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | +| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | +| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | +| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | +| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | +| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | +| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | +| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | +| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | +| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | +| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | +| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | +| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | +| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | +| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +edges +| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | bad-code-sanitization.js:7:31:7:43 | safeProp(key) | +| bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | +| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | +| bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | +| bad-code-sanitization.js:6:11:6:25 | statements | bad-code-sanitization.js:8:27:8:36 | statements | +| bad-code-sanitization.js:6:24:6:25 | [] | bad-code-sanitization.js:6:11:6:25 | statements | +| bad-code-sanitization.js:7:21:7:70 | `${name ... key])}` | bad-code-sanitization.js:6:24:6:25 | [] | +| bad-code-sanitization.js:7:31:7:43 | safeProp(key) | bad-code-sanitization.js:7:21:7:70 | `${name ... key])}` | +| bad-code-sanitization.js:8:27:8:36 | statements | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | +| bad-code-sanitization.js:8:27:8:36 | statements | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | +| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | +| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | +| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | +| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | +| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | +| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | +| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | +#select +| bad-code-sanitization.js:8:27:8:46 | statements.join(';') | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | $@ flows to here and is used to construct code. | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | Improperly sanitized value | +| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | Improperly sanitized value | +| bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:19:27:19:47 | JSON.st ... (input) | Improperly sanitized value | +| bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:31:30:31:50 | JSON.st ... (input) | Improperly sanitized value | +| bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:40:23:40:43 | JSON.st ... (input) | Improperly sanitized value | +| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | Improperly sanitized value | +| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | Improperly sanitized value | diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref new file mode 100644 index 000000000000..20c6dffd7b3e --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.qlref @@ -0,0 +1 @@ +Security/CWE-094/ImproperCodeSanitization.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js new file mode 100644 index 000000000000..ab5a5ff892dd --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js @@ -0,0 +1,55 @@ +function safeProp(key) { + return /^[_$a-zA-Z][_$a-zA-Z0-9]*$/.test(key) ? `.${key}` : `[${JSON.stringify(key)}]`; +} + +function test1() { + const statements = []; + statements.push(`${name}${safeProp(key)}=${stringify(thing[key])}`); + return `(function(){${statements.join(';')}})` // NOT OK +} + +import htmlescape from 'htmlescape' + +function test2(props) { + const pathname = props.data.pathname; + return `function(){return new Error('${htmlescape(pathname)}')}`; // NOT OK +} + +function test3(input) { + return `(function(){${JSON.stringify(input)}))` // NOT OK +} + +function evenSaferProp(key) { + return /^[_$a-zA-Z][_$a-zA-Z0-9]*$/.test(key) ? `.${key}` : `[${JSON.stringify(key)}]`.replace(/[<>\b\f\n\r\t\0\u2028\u2029]/g, ''); +} + +function test4(input) { + return `(function(){${evenSaferProp(input)}))` // OK +} + +function test4(input) { + var foo = `(function(){${JSON.stringify(input)}))` // NOT OK - we can type-track to a code-injection sink, the source is not remote flow. + setTimeout(foo); +} + +function test5(input) { + console.log('methodName() => ' + JSON.stringify(input)); // OK +} + +function test6(input) { + return `(() => {${JSON.stringify(input)})` // NOT OK +} + +function test7(input) { + return `() => {${JSON.stringify(input)}` // NOT OK +} + +var express = require('express'); + +var app = express(); + +app.get('/some/path', function(req, res) { + var foo = `(function(){${JSON.stringify(req.param("wobble"))}))` // NOT - the source is remote-flow, but we know of no sink. + + setTimeout(`(function(){${JSON.stringify(req.param("wobble"))}))`); // OK - the source is remote-flow, and the sink is code-injection. +});