diff --git a/javascript/frameworks/cap/src/loginjection/LogInjection.ql b/javascript/frameworks/cap/src/loginjection/LogInjection.ql index 2b3a76bb9..7e8b3f53a 100644 --- a/javascript/frameworks/cap/src/loginjection/LogInjection.ql +++ b/javascript/frameworks/cap/src/loginjection/LogInjection.ql @@ -1,32 +1,31 @@ /** * @name Uncontrolled data in logging call - * @description If unsanitized user input is written to a log entry, - * a malicious user may be able to forge new log entries - * @kind problem + * @description Building log entries from user-controlled sources is vulnerable to + * insertion of forged log entries by a malicious user. + * @kind path-problem * @problem.severity error - * @id javascript/log-injection-custom + * @security-severity 7.8 + * @precision medium + * @id js/cap-log-injection * @tags security - * external/cwe/cwe-117 */ - import javascript - import advanced_security.javascript.frameworks.cap.CDS - - class LogInjectionConfiguration extends TaintTracking::Configuration { - LogInjectionConfiguration(){ - this = "" - } - override predicate isSource(DataFlow::Node source) { - exists(CDS::RequestSource src | - source = src ) - } - - override predicate isSink(DataFlow::Node sink) { - exists(CDS::CdsLogSink snk | - sink = snk ) - } - } - - from LogInjectionConfiguration log , DataFlow::Node source, DataFlow::Node sink - where log.hasFlow(source, sink) - select sink, "Log injection vulnerability found." \ No newline at end of file +import javascript +import DataFlow::PathGraph +import semmle.javascript.security.dataflow.LogInjectionQuery +import advanced_security.javascript.frameworks.cap.CDS + +/** + * A source of remote user controlled input. + */ +class CapRemoteSource extends Source, CDS::RequestSource { } + +/** + * An argument to a logging mechanism. + */ +class CapLoggingSink extends Sink, CDS::CdsLogSink { } + +from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/frameworks/cap/src/sqlinjection/SqlInjection.ql b/javascript/frameworks/cap/src/sqlinjection/SqlInjection.ql index 3ca9c8cc6..ed3e6a0b5 100644 --- a/javascript/frameworks/cap/src/sqlinjection/SqlInjection.ql +++ b/javascript/frameworks/cap/src/sqlinjection/SqlInjection.ql @@ -1,48 +1,48 @@ /** - * @name Uncontrolled data in SQL query - * @description Including user-supplied data in a SQL query without - * neutralizing special elements can make code vulnerable - * to SQL Injection. - * @kind problem + * @name Database query built from user-controlled sources with additional heuristic sources + * @description Building a database query from user-controlled sources is vulnerable to insertion of + * malicious code by the user. + * @kind path-problem * @problem.severity error - * @id javascript/sql-injection-custom + * @security-severity 8.8 + * @precision high + * @id js/cap-sql-injection * @tags security - * external/cwe/cwe-089 */ import javascript +import DataFlow::PathGraph +import semmle.javascript.security.dataflow.SqlInjectionCustomizations::SqlInjection import advanced_security.javascript.frameworks.cap.CDS import advanced_security.javascript.frameworks.cap.CQL -class SqlInjectionConfiguration extends TaintTracking::Configuration { - SqlInjectionConfiguration(){ - this = "" - } - override predicate isSource(DataFlow::Node source) { - exists(CDS::RequestSource src | - source = src ) - } +class Configuration extends TaintTracking::Configuration { + Configuration() { this = "CapSqlInjection" } - override predicate isSink(DataFlow::Node sink) { - exists(CQL::CQLSink snk | - sink = snk ) - } + override predicate isSource(DataFlow::Node source) { + source instanceof Source or source instanceof CDS::RequestSource + } - override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - //string concatenation in a clause arg taints the clause - exists(CQL::TaintedClause clause | - clause.getArgument() = pred.asExpr() - and clause.asExpr() = succ.asExpr() - ) - or - //less precise, any concat in the alternative sql stmt construction techniques - exists(CQL::ParseCQLTaintedClause parse | - parse.getAnArgument() = pred - and parse = succ - ) - } + override predicate isSink(DataFlow::Node sink) { + sink instanceof Sink or sink instanceof CQL::CQLSink + } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + //string concatenation in a clause arg taints the clause + exists(CQL::TaintedClause clause | + clause.getArgument() = pred.asExpr() and + clause.asExpr() = succ.asExpr() + ) + or + //less precise, any concat in the alternative sql stmt construction techniques + exists(CQL::ParseCQLTaintedClause parse | + parse.getAnArgument() = pred and + parse = succ + ) + } } -from SqlInjectionConfiguration sql , DataFlow::Node source, DataFlow::Node sink -where sql.hasFlow(source, sink) -select sink, "Injection vulnerability found." \ No newline at end of file +from Configuration sql, DataFlow::PathNode source, DataFlow::PathNode sink +where sql.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(), + "user-provided value" diff --git a/javascript/frameworks/cap/test/queries/loginjection/loginjection.expected b/javascript/frameworks/cap/test/queries/loginjection/loginjection.expected index b00bd9217..85a8e0b18 100644 --- a/javascript/frameworks/cap/test/queries/loginjection/loginjection.expected +++ b/javascript/frameworks/cap/test/queries/loginjection/loginjection.expected @@ -1 +1,23 @@ -| loginjection.js:11:14:11:26 | "test" + book | Log injection vulnerability found. | +nodes +| loginjection.js:7:33:7:35 | req | +| loginjection.js:7:33:7:35 | req | +| loginjection.js:8:11:8:25 | {book,quantity} | +| loginjection.js:8:11:8:36 | book | +| loginjection.js:8:12:8:15 | book | +| loginjection.js:8:29:8:31 | req | +| loginjection.js:8:29:8:36 | req.data | +| loginjection.js:11:14:11:26 | "test" + book | +| loginjection.js:11:14:11:26 | "test" + book | +| loginjection.js:11:23:11:26 | book | +edges +| loginjection.js:7:33:7:35 | req | loginjection.js:8:29:8:31 | req | +| loginjection.js:7:33:7:35 | req | loginjection.js:8:29:8:31 | req | +| loginjection.js:8:11:8:25 | {book,quantity} | loginjection.js:8:12:8:15 | book | +| loginjection.js:8:11:8:36 | book | loginjection.js:11:23:11:26 | book | +| loginjection.js:8:12:8:15 | book | loginjection.js:8:11:8:36 | book | +| loginjection.js:8:29:8:31 | req | loginjection.js:8:29:8:36 | req.data | +| loginjection.js:8:29:8:36 | req.data | loginjection.js:8:11:8:25 | {book,quantity} | +| loginjection.js:11:23:11:26 | book | loginjection.js:11:14:11:26 | "test" + book | +| loginjection.js:11:23:11:26 | book | loginjection.js:11:14:11:26 | "test" + book | +#select +| loginjection.js:11:14:11:26 | "test" + book | loginjection.js:7:33:7:35 | req | loginjection.js:11:14:11:26 | "test" + book | Log entry depends on a $@. | loginjection.js:7:33:7:35 | req | user-provided value | diff --git a/javascript/frameworks/cap/test/queries/sqlinjection/sqlinjection.expected b/javascript/frameworks/cap/test/queries/sqlinjection/sqlinjection.expected index 933e1888e..39a9faf48 100644 --- a/javascript/frameworks/cap/test/queries/sqlinjection/sqlinjection.expected +++ b/javascript/frameworks/cap/test/queries/sqlinjection/sqlinjection.expected @@ -1,6 +1,84 @@ -| sqlinjection.js:13:35:13:39 | query | Injection vulnerability found. | -| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | Injection vulnerability found. | -| sqlinjection.js:18:36:18:41 | query2 | Injection vulnerability found. | -| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | Injection vulnerability found. | -| sqlinjection.js:28:38:28:40 | cqn | Injection vulnerability found. | -| sqlinjection.js:31:38:31:41 | cqn1 | Injection vulnerability found. | +nodes +| sqlinjection.js:7:33:7:35 | req | +| sqlinjection.js:7:33:7:35 | req | +| sqlinjection.js:8:11:8:25 | {book,quantity} | +| sqlinjection.js:8:11:8:36 | book | +| sqlinjection.js:8:12:8:15 | book | +| sqlinjection.js:8:29:8:31 | req | +| sqlinjection.js:8:29:8:36 | req.data | +| sqlinjection.js:12:9:12:57 | query | +| sqlinjection.js:12:17:12:57 | SELECT. ... book}`) | +| sqlinjection.js:12:45:12:56 | `ID=${book}` | +| sqlinjection.js:12:51:12:54 | book | +| sqlinjection.js:13:35:13:39 | query | +| sqlinjection.js:13:35:13:39 | query | +| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | +| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | +| sqlinjection.js:15:53:15:64 | `ID=${book}` | +| sqlinjection.js:15:59:15:62 | book | +| sqlinjection.js:17:9:17:56 | query2 | +| sqlinjection.js:17:18:17:56 | SELECT. ... '+book) | +| sqlinjection.js:17:46:17:55 | 'ID='+book | +| sqlinjection.js:17:52:17:55 | book | +| sqlinjection.js:18:36:18:41 | query2 | +| sqlinjection.js:18:36:18:41 | query2 | +| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | +| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | +| sqlinjection.js:20:53:20:62 | 'ID='+book | +| sqlinjection.js:20:59:20:62 | book | +| sqlinjection.js:27:9:27:60 | cqn | +| sqlinjection.js:27:15:27:60 | CQL`SEL ... + book | +| sqlinjection.js:27:57:27:60 | book | +| sqlinjection.js:28:38:28:40 | cqn | +| sqlinjection.js:28:38:28:40 | cqn | +| sqlinjection.js:30:9:30:58 | cqn1 | +| sqlinjection.js:30:16:30:58 | cds.par ... + book) | +| sqlinjection.js:30:31:30:57 | `SELECT ... `+ book | +| sqlinjection.js:30:54:30:57 | book | +| sqlinjection.js:31:38:31:41 | cqn1 | +| sqlinjection.js:31:38:31:41 | cqn1 | +edges +| sqlinjection.js:7:33:7:35 | req | sqlinjection.js:8:29:8:31 | req | +| sqlinjection.js:7:33:7:35 | req | sqlinjection.js:8:29:8:31 | req | +| sqlinjection.js:8:11:8:25 | {book,quantity} | sqlinjection.js:8:12:8:15 | book | +| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:12:51:12:54 | book | +| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:15:59:15:62 | book | +| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:17:52:17:55 | book | +| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:20:59:20:62 | book | +| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:27:57:27:60 | book | +| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:30:54:30:57 | book | +| sqlinjection.js:8:12:8:15 | book | sqlinjection.js:8:11:8:36 | book | +| sqlinjection.js:8:29:8:31 | req | sqlinjection.js:8:29:8:36 | req.data | +| sqlinjection.js:8:29:8:36 | req.data | sqlinjection.js:8:11:8:25 | {book,quantity} | +| sqlinjection.js:12:9:12:57 | query | sqlinjection.js:13:35:13:39 | query | +| sqlinjection.js:12:9:12:57 | query | sqlinjection.js:13:35:13:39 | query | +| sqlinjection.js:12:17:12:57 | SELECT. ... book}`) | sqlinjection.js:12:9:12:57 | query | +| sqlinjection.js:12:45:12:56 | `ID=${book}` | sqlinjection.js:12:17:12:57 | SELECT. ... book}`) | +| sqlinjection.js:12:51:12:54 | book | sqlinjection.js:12:45:12:56 | `ID=${book}` | +| sqlinjection.js:15:53:15:64 | `ID=${book}` | sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | +| sqlinjection.js:15:53:15:64 | `ID=${book}` | sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | +| sqlinjection.js:15:59:15:62 | book | sqlinjection.js:15:53:15:64 | `ID=${book}` | +| sqlinjection.js:17:9:17:56 | query2 | sqlinjection.js:18:36:18:41 | query2 | +| sqlinjection.js:17:9:17:56 | query2 | sqlinjection.js:18:36:18:41 | query2 | +| sqlinjection.js:17:18:17:56 | SELECT. ... '+book) | sqlinjection.js:17:9:17:56 | query2 | +| sqlinjection.js:17:46:17:55 | 'ID='+book | sqlinjection.js:17:18:17:56 | SELECT. ... '+book) | +| sqlinjection.js:17:52:17:55 | book | sqlinjection.js:17:46:17:55 | 'ID='+book | +| sqlinjection.js:20:53:20:62 | 'ID='+book | sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | +| sqlinjection.js:20:53:20:62 | 'ID='+book | sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | +| sqlinjection.js:20:59:20:62 | book | sqlinjection.js:20:53:20:62 | 'ID='+book | +| sqlinjection.js:27:9:27:60 | cqn | sqlinjection.js:28:38:28:40 | cqn | +| sqlinjection.js:27:9:27:60 | cqn | sqlinjection.js:28:38:28:40 | cqn | +| sqlinjection.js:27:15:27:60 | CQL`SEL ... + book | sqlinjection.js:27:9:27:60 | cqn | +| sqlinjection.js:27:57:27:60 | book | sqlinjection.js:27:15:27:60 | CQL`SEL ... + book | +| sqlinjection.js:30:9:30:58 | cqn1 | sqlinjection.js:31:38:31:41 | cqn1 | +| sqlinjection.js:30:9:30:58 | cqn1 | sqlinjection.js:31:38:31:41 | cqn1 | +| sqlinjection.js:30:16:30:58 | cds.par ... + book) | sqlinjection.js:30:9:30:58 | cqn1 | +| sqlinjection.js:30:31:30:57 | `SELECT ... `+ book | sqlinjection.js:30:16:30:58 | cds.par ... + book) | +| sqlinjection.js:30:54:30:57 | book | sqlinjection.js:30:31:30:57 | `SELECT ... `+ book | +#select +| sqlinjection.js:13:35:13:39 | query | sqlinjection.js:7:33:7:35 | req | sqlinjection.js:13:35:13:39 | query | This query depends on a $@. | sqlinjection.js:7:33:7:35 | req | user-provided value | +| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | sqlinjection.js:7:33:7:35 | req | sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | This query depends on a $@. | sqlinjection.js:7:33:7:35 | req | user-provided value | +| sqlinjection.js:18:36:18:41 | query2 | sqlinjection.js:7:33:7:35 | req | sqlinjection.js:18:36:18:41 | query2 | This query depends on a $@. | sqlinjection.js:7:33:7:35 | req | user-provided value | +| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | sqlinjection.js:7:33:7:35 | req | sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | This query depends on a $@. | sqlinjection.js:7:33:7:35 | req | user-provided value | +| sqlinjection.js:28:38:28:40 | cqn | sqlinjection.js:7:33:7:35 | req | sqlinjection.js:28:38:28:40 | cqn | This query depends on a $@. | sqlinjection.js:7:33:7:35 | req | user-provided value | +| sqlinjection.js:31:38:31:41 | cqn1 | sqlinjection.js:7:33:7:35 | req | sqlinjection.js:31:38:31:41 | cqn1 | This query depends on a $@. | sqlinjection.js:7:33:7:35 | req | user-provided value |