Skip to content

Commit 5989d69

Browse files
authored
Merge pull request #82 from advanced-security/mbaluda-cap-queries
CAP SQLi/LOGi queries
2 parents e26a2ed + f15fcc7 commit 5989d69

File tree

4 files changed

+167
-68
lines changed

4 files changed

+167
-68
lines changed
Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,31 @@
11
/**
22
* @name Uncontrolled data in logging call
3-
* @description If unsanitized user input is written to a log entry,
4-
* a malicious user may be able to forge new log entries
5-
* @kind problem
3+
* @description Building log entries from user-controlled sources is vulnerable to
4+
* insertion of forged log entries by a malicious user.
5+
* @kind path-problem
66
* @problem.severity error
7-
* @id javascript/log-injection-custom
7+
* @security-severity 7.8
8+
* @precision medium
9+
* @id js/cap-log-injection
810
* @tags security
9-
* external/cwe/cwe-117
1011
*/
1112

12-
import javascript
13-
import advanced_security.javascript.frameworks.cap.CDS
14-
15-
class LogInjectionConfiguration extends TaintTracking::Configuration {
16-
LogInjectionConfiguration(){
17-
this = ""
18-
}
19-
override predicate isSource(DataFlow::Node source) {
20-
exists(CDS::RequestSource src |
21-
source = src )
22-
}
23-
24-
override predicate isSink(DataFlow::Node sink) {
25-
exists(CDS::CdsLogSink snk |
26-
sink = snk )
27-
}
28-
}
29-
30-
from LogInjectionConfiguration log , DataFlow::Node source, DataFlow::Node sink
31-
where log.hasFlow(source, sink)
32-
select sink, "Log injection vulnerability found."
13+
import javascript
14+
import DataFlow::PathGraph
15+
import semmle.javascript.security.dataflow.LogInjectionQuery
16+
import advanced_security.javascript.frameworks.cap.CDS
17+
18+
/**
19+
* A source of remote user controlled input.
20+
*/
21+
class CapRemoteSource extends Source, CDS::RequestSource { }
22+
23+
/**
24+
* An argument to a logging mechanism.
25+
*/
26+
class CapLoggingSink extends Sink, CDS::CdsLogSink { }
27+
28+
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
29+
where config.hasFlowPath(source, sink)
30+
select sink.getNode(), source, sink, "Log entry depends on a $@.", source.getNode(),
31+
"user-provided value"
Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,48 @@
11
/**
2-
* @name Uncontrolled data in SQL query
3-
* @description Including user-supplied data in a SQL query without
4-
* neutralizing special elements can make code vulnerable
5-
* to SQL Injection.
6-
* @kind problem
2+
* @name Database query built from user-controlled sources with additional heuristic sources
3+
* @description Building a database query from user-controlled sources is vulnerable to insertion of
4+
* malicious code by the user.
5+
* @kind path-problem
76
* @problem.severity error
8-
* @id javascript/sql-injection-custom
7+
* @security-severity 8.8
8+
* @precision high
9+
* @id js/cap-sql-injection
910
* @tags security
10-
* external/cwe/cwe-089
1111
*/
1212

1313
import javascript
14+
import DataFlow::PathGraph
15+
import semmle.javascript.security.dataflow.SqlInjectionCustomizations::SqlInjection
1416
import advanced_security.javascript.frameworks.cap.CDS
1517
import advanced_security.javascript.frameworks.cap.CQL
1618

17-
class SqlInjectionConfiguration extends TaintTracking::Configuration {
18-
SqlInjectionConfiguration(){
19-
this = ""
20-
}
21-
override predicate isSource(DataFlow::Node source) {
22-
exists(CDS::RequestSource src |
23-
source = src )
24-
}
19+
class Configuration extends TaintTracking::Configuration {
20+
Configuration() { this = "CapSqlInjection" }
2521

26-
override predicate isSink(DataFlow::Node sink) {
27-
exists(CQL::CQLSink snk |
28-
sink = snk )
29-
}
22+
override predicate isSource(DataFlow::Node source) {
23+
source instanceof Source or source instanceof CDS::RequestSource
24+
}
3025

31-
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
32-
//string concatenation in a clause arg taints the clause
33-
exists(CQL::TaintedClause clause |
34-
clause.getArgument() = pred.asExpr()
35-
and clause.asExpr() = succ.asExpr()
36-
)
37-
or
38-
//less precise, any concat in the alternative sql stmt construction techniques
39-
exists(CQL::ParseCQLTaintedClause parse |
40-
parse.getAnArgument() = pred
41-
and parse = succ
42-
)
43-
}
26+
override predicate isSink(DataFlow::Node sink) {
27+
sink instanceof Sink or sink instanceof CQL::CQLSink
28+
}
29+
30+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
31+
//string concatenation in a clause arg taints the clause
32+
exists(CQL::TaintedClause clause |
33+
clause.getArgument() = pred.asExpr() and
34+
clause.asExpr() = succ.asExpr()
35+
)
36+
or
37+
//less precise, any concat in the alternative sql stmt construction techniques
38+
exists(CQL::ParseCQLTaintedClause parse |
39+
parse.getAnArgument() = pred and
40+
parse = succ
41+
)
42+
}
4443
}
4544

46-
from SqlInjectionConfiguration sql , DataFlow::Node source, DataFlow::Node sink
47-
where sql.hasFlow(source, sink)
48-
select sink, "Injection vulnerability found."
45+
from Configuration sql, DataFlow::PathNode source, DataFlow::PathNode sink
46+
where sql.hasFlowPath(source, sink)
47+
select sink.getNode(), source, sink, "This query depends on a $@.", source.getNode(),
48+
"user-provided value"
Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,23 @@
1-
| loginjection.js:11:14:11:26 | "test" + book | Log injection vulnerability found. |
1+
nodes
2+
| loginjection.js:7:33:7:35 | req |
3+
| loginjection.js:7:33:7:35 | req |
4+
| loginjection.js:8:11:8:25 | {book,quantity} |
5+
| loginjection.js:8:11:8:36 | book |
6+
| loginjection.js:8:12:8:15 | book |
7+
| loginjection.js:8:29:8:31 | req |
8+
| loginjection.js:8:29:8:36 | req.data |
9+
| loginjection.js:11:14:11:26 | "test" + book |
10+
| loginjection.js:11:14:11:26 | "test" + book |
11+
| loginjection.js:11:23:11:26 | book |
12+
edges
13+
| loginjection.js:7:33:7:35 | req | loginjection.js:8:29:8:31 | req |
14+
| loginjection.js:7:33:7:35 | req | loginjection.js:8:29:8:31 | req |
15+
| loginjection.js:8:11:8:25 | {book,quantity} | loginjection.js:8:12:8:15 | book |
16+
| loginjection.js:8:11:8:36 | book | loginjection.js:11:23:11:26 | book |
17+
| loginjection.js:8:12:8:15 | book | loginjection.js:8:11:8:36 | book |
18+
| loginjection.js:8:29:8:31 | req | loginjection.js:8:29:8:36 | req.data |
19+
| loginjection.js:8:29:8:36 | req.data | loginjection.js:8:11:8:25 | {book,quantity} |
20+
| loginjection.js:11:23:11:26 | book | loginjection.js:11:14:11:26 | "test" + book |
21+
| loginjection.js:11:23:11:26 | book | loginjection.js:11:14:11:26 | "test" + book |
22+
#select
23+
| 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 |
Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,84 @@
1-
| sqlinjection.js:13:35:13:39 | query | Injection vulnerability found. |
2-
| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) | Injection vulnerability found. |
3-
| sqlinjection.js:18:36:18:41 | query2 | Injection vulnerability found. |
4-
| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) | Injection vulnerability found. |
5-
| sqlinjection.js:28:38:28:40 | cqn | Injection vulnerability found. |
6-
| sqlinjection.js:31:38:31:41 | cqn1 | Injection vulnerability found. |
1+
nodes
2+
| sqlinjection.js:7:33:7:35 | req |
3+
| sqlinjection.js:7:33:7:35 | req |
4+
| sqlinjection.js:8:11:8:25 | {book,quantity} |
5+
| sqlinjection.js:8:11:8:36 | book |
6+
| sqlinjection.js:8:12:8:15 | book |
7+
| sqlinjection.js:8:29:8:31 | req |
8+
| sqlinjection.js:8:29:8:36 | req.data |
9+
| sqlinjection.js:12:9:12:57 | query |
10+
| sqlinjection.js:12:17:12:57 | SELECT. ... book}`) |
11+
| sqlinjection.js:12:45:12:56 | `ID=${book}` |
12+
| sqlinjection.js:12:51:12:54 | book |
13+
| sqlinjection.js:13:35:13:39 | query |
14+
| sqlinjection.js:13:35:13:39 | query |
15+
| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) |
16+
| sqlinjection.js:15:25:15:65 | SELECT. ... book}`) |
17+
| sqlinjection.js:15:53:15:64 | `ID=${book}` |
18+
| sqlinjection.js:15:59:15:62 | book |
19+
| sqlinjection.js:17:9:17:56 | query2 |
20+
| sqlinjection.js:17:18:17:56 | SELECT. ... '+book) |
21+
| sqlinjection.js:17:46:17:55 | 'ID='+book |
22+
| sqlinjection.js:17:52:17:55 | book |
23+
| sqlinjection.js:18:36:18:41 | query2 |
24+
| sqlinjection.js:18:36:18:41 | query2 |
25+
| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) |
26+
| sqlinjection.js:20:25:20:63 | SELECT. ... '+book) |
27+
| sqlinjection.js:20:53:20:62 | 'ID='+book |
28+
| sqlinjection.js:20:59:20:62 | book |
29+
| sqlinjection.js:27:9:27:60 | cqn |
30+
| sqlinjection.js:27:15:27:60 | CQL`SEL ... + book |
31+
| sqlinjection.js:27:57:27:60 | book |
32+
| sqlinjection.js:28:38:28:40 | cqn |
33+
| sqlinjection.js:28:38:28:40 | cqn |
34+
| sqlinjection.js:30:9:30:58 | cqn1 |
35+
| sqlinjection.js:30:16:30:58 | cds.par ... + book) |
36+
| sqlinjection.js:30:31:30:57 | `SELECT ... `+ book |
37+
| sqlinjection.js:30:54:30:57 | book |
38+
| sqlinjection.js:31:38:31:41 | cqn1 |
39+
| sqlinjection.js:31:38:31:41 | cqn1 |
40+
edges
41+
| sqlinjection.js:7:33:7:35 | req | sqlinjection.js:8:29:8:31 | req |
42+
| sqlinjection.js:7:33:7:35 | req | sqlinjection.js:8:29:8:31 | req |
43+
| sqlinjection.js:8:11:8:25 | {book,quantity} | sqlinjection.js:8:12:8:15 | book |
44+
| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:12:51:12:54 | book |
45+
| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:15:59:15:62 | book |
46+
| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:17:52:17:55 | book |
47+
| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:20:59:20:62 | book |
48+
| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:27:57:27:60 | book |
49+
| sqlinjection.js:8:11:8:36 | book | sqlinjection.js:30:54:30:57 | book |
50+
| sqlinjection.js:8:12:8:15 | book | sqlinjection.js:8:11:8:36 | book |
51+
| sqlinjection.js:8:29:8:31 | req | sqlinjection.js:8:29:8:36 | req.data |
52+
| sqlinjection.js:8:29:8:36 | req.data | sqlinjection.js:8:11:8:25 | {book,quantity} |
53+
| sqlinjection.js:12:9:12:57 | query | sqlinjection.js:13:35:13:39 | query |
54+
| sqlinjection.js:12:9:12:57 | query | sqlinjection.js:13:35:13:39 | query |
55+
| sqlinjection.js:12:17:12:57 | SELECT. ... book}`) | sqlinjection.js:12:9:12:57 | query |
56+
| sqlinjection.js:12:45:12:56 | `ID=${book}` | sqlinjection.js:12:17:12:57 | SELECT. ... book}`) |
57+
| sqlinjection.js:12:51:12:54 | book | sqlinjection.js:12:45:12:56 | `ID=${book}` |
58+
| sqlinjection.js:15:53:15:64 | `ID=${book}` | sqlinjection.js:15:25:15:65 | SELECT. ... book}`) |
59+
| sqlinjection.js:15:53:15:64 | `ID=${book}` | sqlinjection.js:15:25:15:65 | SELECT. ... book}`) |
60+
| sqlinjection.js:15:59:15:62 | book | sqlinjection.js:15:53:15:64 | `ID=${book}` |
61+
| sqlinjection.js:17:9:17:56 | query2 | sqlinjection.js:18:36:18:41 | query2 |
62+
| sqlinjection.js:17:9:17:56 | query2 | sqlinjection.js:18:36:18:41 | query2 |
63+
| sqlinjection.js:17:18:17:56 | SELECT. ... '+book) | sqlinjection.js:17:9:17:56 | query2 |
64+
| sqlinjection.js:17:46:17:55 | 'ID='+book | sqlinjection.js:17:18:17:56 | SELECT. ... '+book) |
65+
| sqlinjection.js:17:52:17:55 | book | sqlinjection.js:17:46:17:55 | 'ID='+book |
66+
| sqlinjection.js:20:53:20:62 | 'ID='+book | sqlinjection.js:20:25:20:63 | SELECT. ... '+book) |
67+
| sqlinjection.js:20:53:20:62 | 'ID='+book | sqlinjection.js:20:25:20:63 | SELECT. ... '+book) |
68+
| sqlinjection.js:20:59:20:62 | book | sqlinjection.js:20:53:20:62 | 'ID='+book |
69+
| sqlinjection.js:27:9:27:60 | cqn | sqlinjection.js:28:38:28:40 | cqn |
70+
| sqlinjection.js:27:9:27:60 | cqn | sqlinjection.js:28:38:28:40 | cqn |
71+
| sqlinjection.js:27:15:27:60 | CQL`SEL ... + book | sqlinjection.js:27:9:27:60 | cqn |
72+
| sqlinjection.js:27:57:27:60 | book | sqlinjection.js:27:15:27:60 | CQL`SEL ... + book |
73+
| sqlinjection.js:30:9:30:58 | cqn1 | sqlinjection.js:31:38:31:41 | cqn1 |
74+
| sqlinjection.js:30:9:30:58 | cqn1 | sqlinjection.js:31:38:31:41 | cqn1 |
75+
| sqlinjection.js:30:16:30:58 | cds.par ... + book) | sqlinjection.js:30:9:30:58 | cqn1 |
76+
| sqlinjection.js:30:31:30:57 | `SELECT ... `+ book | sqlinjection.js:30:16:30:58 | cds.par ... + book) |
77+
| sqlinjection.js:30:54:30:57 | book | sqlinjection.js:30:31:30:57 | `SELECT ... `+ book |
78+
#select
79+
| 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 |
80+
| 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 |
81+
| 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 |
82+
| 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 |
83+
| 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 |
84+
| 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 |

0 commit comments

Comments
 (0)