Skip to content

Commit b2e9555

Browse files
aschackmullaschackmull
authored
Merge pull request #8345 from jorgectf/mybatis-new-sinks
Java: Add `MyBatis`' `Providers` sinks
2 parents 811a2c0 + 37b051a commit b2e9555

File tree

15 files changed

+508
-1
lines changed

15 files changed

+508
-1
lines changed

‎java/ql/lib/change-notes/2022-03-15-mybatis-providers.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added modeling of MyBatis (`org.apache.ibatis`) Providers, resulting in additional sinks for the queries `java/ognl-injection`, `java/sql-injection`, `java/sql-injection-local` and `java/concatenated-sql-query`.

‎java/ql/lib/semmle/code/java/frameworks/MyBatis.qll

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
*/
44

55
import java
6+
private import semmle.code.java.dataflow.DataFlow
7+
private import semmle.code.java.dataflow.TaintTracking
68
private import semmle.code.java.dataflow.ExternalFlow
79

810
/** The class `org.apache.ibatis.jdbc.SqlRunner`. */
@@ -102,3 +104,116 @@ class MyBatisSqlOperationAnnotationMethod extends Method {
102104
class TypeParam extends Interface {
103105
TypeParam() { this.hasQualifiedName("org.apache.ibatis.annotations", "Param") }
104106
}
107+
108+
private class MyBatisProvider extends RefType {
109+
MyBatisProvider() {
110+
this.hasQualifiedName("org.apache.ibatis.annotations",
111+
["Select", "Delete", "Insert", "Update"] + "Provider")
112+
}
113+
}
114+
115+
/**
116+
* A return statement of a method used in a MyBatis Provider.
117+
*
118+
* See
119+
* - `MyBatisProvider`
120+
* - https://mybatis.org/mybatis-3/apidocs/org/apache/ibatis/annotations/package-summary.html
121+
*/
122+
class MyBatisInjectionSink extends DataFlow::Node {
123+
MyBatisInjectionSink() {
124+
exists(Annotation a, Method m |
125+
a.getType() instanceof MyBatisProvider and
126+
m.getDeclaringType() = a.getValue(["type", "value"]).(TypeLiteral).getTypeName().getType() and
127+
m.hasName(a.getValue("method").(StringLiteral).getValue()) and
128+
exists(ReturnStmt ret | this.asExpr() = ret.getResult() and ret.getEnclosingCallable() = m)
129+
)
130+
}
131+
}
132+
133+
private class MyBatisProviderStep extends TaintTracking::AdditionalValueStep {
134+
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
135+
exists(MethodAccess ma, Annotation a, Method providerMethod |
136+
exists(int i |
137+
ma.getArgument(pragma[only_bind_into](i)) = n1.asExpr() and
138+
providerMethod.getParameter(pragma[only_bind_into](i)) = n2.asParameter()
139+
)
140+
|
141+
a.getType() instanceof MyBatisProvider and
142+
ma.getMethod().getAnAnnotation() = a and
143+
providerMethod.getDeclaringType() =
144+
a.getValue(["type", "value"]).(TypeLiteral).getTypeName().getType() and
145+
providerMethod.hasName(a.getValue("method").(StringLiteral).getValue())
146+
)
147+
}
148+
}
149+
150+
private class MyBatisAbstractSqlToStringStep extends SummaryModelCsv {
151+
override predicate row(string row) {
152+
row = "org.apache.ibatis.jdbc;AbstractSQL;true;toString;;;Argument[-1];ReturnValue;taint"
153+
}
154+
}
155+
156+
private class MyBatisAbstractSqlMethodsStep extends SummaryModelCsv {
157+
override predicate row(string row) {
158+
row =
159+
[
160+
"org.apache.ibatis.jdbc;AbstractSQL;true;toString;;;Argument[-1];ReturnValue;taint",
161+
"org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String[]);;Argument[0];Argument[-1];taint",
162+
"org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
163+
"org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String);;Argument[0];Argument[-1];taint",
164+
"org.apache.ibatis.jdbc;AbstractSQL;true;WHERE;(String);;Argument[0].ArrayElement;Argument[-1];taint",
165+
"org.apache.ibatis.jdbc;AbstractSQL;true;VALUES;(String,String);;Argument[0..1];Argument[-1];taint",
166+
"org.apache.ibatis.jdbc;AbstractSQL;true;UPDATE;(String);;Argument[0];Argument[-1];taint",
167+
"org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String[]);;Argument[0];Argument[-1];taint",
168+
"org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
169+
"org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String);;Argument[0];Argument[-1];taint",
170+
"org.apache.ibatis.jdbc;AbstractSQL;true;SET;(String);;Argument[0].ArrayElement;Argument[-1];taint",
171+
"org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String[]);;Argument[0];Argument[-1];taint",
172+
"org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
173+
"org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String);;Argument[0];Argument[-1];taint",
174+
"org.apache.ibatis.jdbc;AbstractSQL;true;SELECT_DISTINCT;(String);;Argument[0].ArrayElement;Argument[-1];taint",
175+
"org.apache.ibatis.jdbc;AbstractSQL;true;SELECT;(String);;Argument[0];Argument[-1];taint",
176+
"org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
177+
"org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
178+
"org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String);;Argument[0];Argument[-1];taint",
179+
"org.apache.ibatis.jdbc;AbstractSQL;true;RIGHT_OUTER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
180+
"org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
181+
"org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
182+
"org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String);;Argument[0];Argument[-1];taint",
183+
"org.apache.ibatis.jdbc;AbstractSQL;true;OUTER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
184+
"org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String[]);;Argument[0];Argument[-1];taint",
185+
"org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
186+
"org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String);;Argument[0];Argument[-1];taint",
187+
"org.apache.ibatis.jdbc;AbstractSQL;true;ORDER_BY;(String);;Argument[0].ArrayElement;Argument[-1];taint",
188+
"org.apache.ibatis.jdbc;AbstractSQL;true;OFFSET_ROWS;(String);;Argument[0];Argument[-1];taint",
189+
"org.apache.ibatis.jdbc;AbstractSQL;true;OFFSET;(String);;Argument[0];Argument[-1];taint",
190+
"org.apache.ibatis.jdbc;AbstractSQL;true;LIMIT;(String);;Argument[0];Argument[-1];taint",
191+
"org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
192+
"org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
193+
"org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String);;Argument[0];Argument[-1];taint",
194+
"org.apache.ibatis.jdbc;AbstractSQL;true;LEFT_OUTER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
195+
"org.apache.ibatis.jdbc;AbstractSQL;true;JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
196+
"org.apache.ibatis.jdbc;AbstractSQL;true;INTO_VALUES;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
197+
"org.apache.ibatis.jdbc;AbstractSQL;true;INTO_COLUMNS;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
198+
"org.apache.ibatis.jdbc;AbstractSQL;true;INSERT_INTO;(String);;Argument[0];Argument[-1];taint",
199+
"org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String[]);;Argument[0];Argument[-1];taint",
200+
"org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
201+
"org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String);;Argument[0];Argument[-1];taint",
202+
"org.apache.ibatis.jdbc;AbstractSQL;true;INNER_JOIN;(String);;Argument[0].ArrayElement;Argument[-1];taint",
203+
"org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String[]);;Argument[0];Argument[-1];taint",
204+
"org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
205+
"org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String);;Argument[0];Argument[-1];taint",
206+
"org.apache.ibatis.jdbc;AbstractSQL;true;HAVING;(String);;Argument[0].ArrayElement;Argument[-1];taint",
207+
"org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String[]);;Argument[0];Argument[-1];taint",
208+
"org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
209+
"org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String);;Argument[0];Argument[-1];taint",
210+
"org.apache.ibatis.jdbc;AbstractSQL;true;GROUP_BY;(String);;Argument[0].ArrayElement;Argument[-1];taint",
211+
"org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String[]);;Argument[0];Argument[-1];taint",
212+
"org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String[]);;Argument[0].ArrayElement;Argument[-1];taint",
213+
"org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String);;Argument[0];Argument[-1];taint",
214+
"org.apache.ibatis.jdbc;AbstractSQL;true;FROM;(String);;Argument[0].ArrayElement;Argument[-1];taint",
215+
"org.apache.ibatis.jdbc;AbstractSQL;true;FETCH_FIRST_ROWS_ONLY;(String);;Argument[0];Argument[-1];taint",
216+
"org.apache.ibatis.jdbc;AbstractSQL;true;DELETE_FROM;(String);;Argument[0];Argument[-1];taint"
217+
]
218+
}
219+
}

‎java/ql/lib/semmle/code/java/security/OgnlInjection.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java
44
private import semmle.code.java.dataflow.DataFlow
55
private import semmle.code.java.dataflow.ExternalFlow
6+
private import semmle.code.java.frameworks.MyBatis
67

78
/**
89
* A data flow sink for unvalidated user input that is used in OGNL EL evaluation.
@@ -122,3 +123,5 @@ private class DefaultOgnlInjectionAdditionalTaintStep extends OgnlInjectionAddit
122123
setExpressionStep(node1, node2)
123124
}
124125
}
126+
127+
private class MyBatisOgnlInjectionSink extends OgnlInjectionSink instanceof MyBatisInjectionSink { }

‎java/ql/lib/semmle/code/java/security/QueryInjection.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java
44
import semmle.code.java.dataflow.DataFlow
55
import semmle.code.java.frameworks.javaee.Persistence
6+
private import semmle.code.java.frameworks.MyBatis
67
import semmle.code.java.dataflow.ExternalFlow
78

89
/** A sink for database query language injection vulnerabilities. */
@@ -66,3 +67,5 @@ private class MongoJsonStep extends AdditionalQueryInjectionTaintStep {
6667
)
6768
}
6869
}
70+
71+
private class MyBatisSqlInjectionSink extends QueryInjectionSink instanceof MyBatisInjectionSink { }

‎java/ql/test/experimental/query-tests/security/CWE-089/src/main/MyBatisAnnotationSqlInjection.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@ nodes
1313
| MybatisSqlInjectionService.java:51:27:51:33 | hashMap | semmle.label | hashMap |
1414
subpaths
1515
#select
16-
| MybatisSqlInjectionService.java:51:27:51:33 | hashMap | MybatisSqlInjection.java:62:19:62:43 | name : String | MybatisSqlInjectionService.java:51:27:51:33 | hashMap | MyBatis annotation SQL injection might include code from $@ to $@. | MybatisSqlInjection.java:62:19:62:43 | name | this user input | SqlInjectionMapper.java:29:2:29:54 | Select | this SQL operation |
16+
| MybatisSqlInjectionService.java:51:27:51:33 | hashMap | MybatisSqlInjection.java:62:19:62:43 | name : String | MybatisSqlInjectionService.java:51:27:51:33 | hashMap | MyBatis annotation SQL injection might include code from $@ to $@. | MybatisSqlInjection.java:62:19:62:43 | name | this user input | SqlInjectionMapper.java:33:2:33:54 | Select | this SQL operation |

‎java/ql/test/experimental/query-tests/security/CWE-089/src/main/MyBatisProvider.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import org.apache.ibatis.annotations.Param;
2+
import org.apache.ibatis.jdbc.SQL;
3+
4+
public class MyBatisProvider {
5+
public String badSelect(@Param("input") final String input) {
6+
String s = (new SQL() {
7+
{
8+
this.SELECT("password");
9+
this.FROM("users");
10+
this.WHERE("username = '" + input + "'");
11+
}
12+
}).toString();
13+
return s;
14+
}
15+
16+
public String badDelete(@Param("input") final String input) {
17+
return "DELETE FROM users WHERE username = '" + input + "';";
18+
}
19+
20+
public String badUpdate(@Param("input") final String input) {
21+
String s = (new SQL() {
22+
{
23+
this.UPDATE("users");
24+
this.SET("balance = 0");
25+
this.WHERE("username = '" + input + "'");
26+
}
27+
}).toString();
28+
return s;
29+
}
30+
31+
public String badInsert(@Param("input") final String input) {
32+
return "INSERT INTO users VALUES (1, '" + input + "', 'hunter2');";
33+
}
34+
}
35+

‎java/ql/test/experimental/query-tests/security/CWE-089/src/main/MybatisSqlInjection.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,25 @@ public List<Test> good1(Integer id) {
6868
List<Test> result = mybatisSqlInjectionService.good1(id);
6969
return result;
7070
}
71+
72+
// using providers
73+
@GetMapping(value = "badSelect")
74+
public String badSelect(@RequestParam String name) {
75+
return mybatisSqlInjectionService.badSelect(name);
76+
}
77+
78+
@GetMapping(value = "badDelete")
79+
public void badDelete(@RequestParam String name) {
80+
mybatisSqlInjectionService.badDelete(name);
81+
}
82+
83+
@GetMapping(value = "badUpdate")
84+
public void badUpdate(@RequestParam String name) {
85+
mybatisSqlInjectionService.badUpdate(name);
86+
}
87+
88+
@GetMapping(value = "badInsert")
89+
public void badInsert(@RequestParam String name) {
90+
mybatisSqlInjectionService.badInsert(name);
91+
}
7192
}

‎java/ql/test/experimental/query-tests/security/CWE-089/src/main/MybatisSqlInjectionService.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,21 @@ public List<Test> good1(Integer id) {
5555
List<Test> result = sqlInjectionMapper.good1(id);
5656
return result;
5757
}
58+
59+
// using providers
60+
public String badSelect(String input) {
61+
return sqlInjectionMapper.badSelect(input);
62+
}
63+
64+
public void badDelete(String input) {
65+
sqlInjectionMapper.badDelete(input);
66+
}
67+
68+
public void badUpdate(String input) {
69+
sqlInjectionMapper.badUpdate(input);
70+
}
71+
72+
public void badInsert(String input) {
73+
sqlInjectionMapper.badInsert(input);
74+
}
5875
}

‎java/ql/test/experimental/query-tests/security/CWE-089/src/main/SqlInjectionMapper.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
import org.apache.ibatis.annotations.Param;
66
import org.springframework.stereotype.Repository;
77
import org.apache.ibatis.annotations.Select;
8+
import org.apache.ibatis.annotations.SelectProvider;
9+
import org.apache.ibatis.annotations.DeleteProvider;
10+
import org.apache.ibatis.annotations.UpdateProvider;
11+
import org.apache.ibatis.annotations.InsertProvider;
812

913
@Mapper
1014
@Repository
@@ -30,4 +34,29 @@ public interface SqlInjectionMapper {
3034
public Test bad9(HashMap<String, Object> map);
3135

3236
List<Test> good1(Integer id);
37+
38+
//using providers
39+
@SelectProvider(
40+
type = MyBatisProvider.class,
41+
method = "badSelect"
42+
)
43+
String badSelect(String input);
44+
45+
@DeleteProvider(
46+
type = MyBatisProvider.class,
47+
method = "badDelete"
48+
)
49+
void badDelete(String input);
50+
51+
@UpdateProvider(
52+
type = MyBatisProvider.class,
53+
method = "badUpdate"
54+
)
55+
void badUpdate(String input);
56+
57+
@InsertProvider(
58+
type = MyBatisProvider.class,
59+
method = "badInsert"
60+
)
61+
void badInsert(String input);
3362
}

‎java/ql/test/stubs/org.mybatis-3.5.4/org/apache/ibatis/annotations/DeleteProvider.java

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)