Skip to content

Commit 69690a2

Browse files
haby0haby0
committed
Modify sinks
1 parent 4438f8c commit 69690a2

File tree

9 files changed

+176
-63
lines changed

9 files changed

+176
-63
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,14 @@ import DataFlow::PathGraph
1616
import MyBatisMapperXmlSqlInjectionLib
1717
import semmle.code.java.dataflow.FlowSources
1818

19-
/**
20-
* A taint-tracking configuration for tracking untrusted user input used by the Mybatis mapper xml file dynamic splicing sql use.
21-
*/
2219
private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::Configuration {
2320
MyBatisMapperXmlSqlInjectionConfiguration() { this = "MyBatis mapper xml sql injection" }
2421

2522
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2623

27-
override predicate isSink(DataFlow::Node sink) { sink instanceof MyBatisMapperXmlSqlInjectionSink }
24+
override predicate isSink(DataFlow::Node sink) {
25+
sink instanceof MyBatisMapperXmlSqlInjectionSink
26+
}
2827

2928
override predicate isSanitizer(DataFlow::Node node) {
3029
node.getType() instanceof PrimitiveType or
@@ -33,7 +32,8 @@ private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::C
3332
}
3433
}
3534

36-
from MyBatisMapperXmlSqlInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
35+
from
36+
MyBatisMapperXmlSqlInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
3737
where cfg.hasFlowPath(source, sink)
38-
select sink.getNode(), source, sink, "MyBatis Mapper XML sql injection might include code from $@.", source.getNode(),
39-
"this user input"
38+
select sink.getNode(), source, sink, "MyBatis Mapper XML sql injection might include code from $@.",
39+
source.getNode(), "this user input"

‎java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjectionLib.qll

Lines changed: 88 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,100 @@ import java
22
import semmle.code.xml.MyBatisMapperXML
33
import semmle.code.java.dataflow.FlowSources
44

5+
/** The interface `org.apache.ibatis.annotations.Param`. */
6+
private class TypeParam extends Interface {
7+
TypeParam() { this.hasQualifiedName("org.apache.ibatis.annotations", "Param") }
8+
}
9+
510
/** A sink for MyBatis Mapper XML file sql injection vulnerabilities. */
6-
class MyBatisMapperXmlSqlInjectionSink extends DataFlow::Node {
7-
MyBatisMapperXmlSqlInjectionSink() {
8-
exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma |
9-
mbmxe.getAChild*().getTextValue().trim().matches("%${%") and
11+
abstract class MyBatisMapperXmlSqlInjectionSink extends DataFlow::Node { }
12+
13+
/**
14+
* A sink for MyBatis Mapper method parameter name sql injection vulnerabilities.
15+
*
16+
* e.g. MyBatis Mapper method: `void test(String name);` and MyBatis Mapper XML file:`select id,name from test where name like '%${name}%'`
17+
*/
18+
class MyBatisMapperParameterNameSqlInjectionSink extends MyBatisMapperXmlSqlInjectionSink {
19+
MyBatisMapperParameterNameSqlInjectionSink() {
20+
exists(
21+
MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess ma, int i, Method m,
22+
Expr arg, string sql
23+
|
24+
m = ma.getMethod() and arg = ma.getArgument(i)
25+
|
26+
arg = this.asExpr() and
27+
(
28+
mbmxe.getAChild*().getTextValue().trim() = sql
29+
or
30+
mbmxe.getInclude().getRefid() = mbms.getId() and
31+
mbms.getAChild*().getTextValue().trim() = sql
32+
) and
33+
not m.getParameter(i).hasAnnotation() and
34+
sql.matches("%${" + m.getParameter(i).getName() + "%") and
35+
mbmxe.getId() = ma.getMethod().getName() and
36+
ma.getMethod().getDeclaringType() =
37+
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType()
38+
)
39+
}
40+
}
41+
42+
/**
43+
* A sink for MyBatis Mapper method Param Annotation sql injection vulnerabilities.
44+
*
45+
* e.g. MyBatis Mapper method: `void test(@Param("orderby") String name);` and MyBatis Mapper XML file:`select id,name from test order by ${orderby,jdbcType=VARCHAR}`
46+
*/
47+
class MyBatisMapperParamAnnotationSqlInjectionSink extends MyBatisMapperXmlSqlInjectionSink {
48+
MyBatisMapperParamAnnotationSqlInjectionSink() {
49+
exists(
50+
MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess ma, int i, Method m,
51+
Expr arg, Annotation a, string sql
52+
|
53+
m = ma.getMethod() and arg = ma.getArgument(i)
54+
|
55+
arg = this.asExpr() and
56+
(
57+
mbmxe.getAChild*().getTextValue().trim() = sql
58+
or
59+
mbmxe.getInclude().getRefid() = mbms.getId() and
60+
mbms.getAChild*().getTextValue().trim() = sql
61+
) and
62+
m.getParameter(i).hasAnnotation() and
63+
m.getParameter(i).getAnAnnotation() = a and
64+
a.getType() instanceof TypeParam and
65+
sql.matches("%${" + a.getValue("value").(CompileTimeConstantExpr).getStringValue() + "%") and
1066
mbmxe.getId() = ma.getMethod().getName() and
1167
ma.getMethod().getDeclaringType() =
12-
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType() and
13-
ma.getAnArgument() = this.asExpr()
68+
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType()
1469
)
15-
or
16-
exists(MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess ma |
17-
mbmxe.getInclude().getRefid() = mbms.getId() and
18-
mbms.getAChild*().getTextValue().trim().matches("%${%") and
70+
}
71+
}
72+
73+
/**
74+
* A sink for MyBatis Mapper method Class Field sql injection vulnerabilities.
75+
*
76+
* e.g. MyBatis Mapper method: `void test(Test test);` and MyBatis Mapper XML file:`select id,name from test order by ${name,jdbcType=VARCHAR}`
77+
*/
78+
class MyBatisMapperClassFieldSqlInjectionSink extends MyBatisMapperXmlSqlInjectionSink {
79+
MyBatisMapperClassFieldSqlInjectionSink() {
80+
exists(
81+
MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess ma, int i, Method m,
82+
Expr arg, string sql, Class c
83+
|
84+
m = ma.getMethod() and arg = ma.getArgument(i)
85+
|
86+
arg = this.asExpr() and
87+
(
88+
mbmxe.getAChild*().getTextValue().trim() = sql
89+
or
90+
mbmxe.getInclude().getRefid() = mbms.getId() and
91+
mbms.getAChild*().getTextValue().trim() = sql
92+
) and
93+
not m.getParameter(i).hasAnnotation() and
94+
m.getParameterType(i).getName() = c.getName() and
95+
sql.matches("%${" + c.getAField().getName() + "%") and
1996
mbmxe.getId() = ma.getMethod().getName() and
2097
ma.getMethod().getDeclaringType() =
21-
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType() and
22-
ma.getAnArgument() = this.asExpr()
98+
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType()
2399
)
24100
}
25101
}

‎java/ql/test/experimental/query-tests/security/CWE-089/MyBatisMapperXmlSqlInjection.expected

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,43 @@
11
edges
2-
| MybatisSqlInjection.java:16:25:16:35 | name : String | MybatisSqlInjection.java:17:55:17:58 | name : String |
3-
| MybatisSqlInjection.java:17:55:17:58 | name : String | MybatisSqlInjectionService.java:11:25:11:35 | name : String |
4-
| MybatisSqlInjection.java:22:25:22:35 | name : String | MybatisSqlInjection.java:23:55:23:58 | name : String |
5-
| MybatisSqlInjection.java:23:55:23:58 | name : String | MybatisSqlInjectionService.java:16:25:16:35 | name : String |
6-
| MybatisSqlInjection.java:28:25:28:35 | name : String | MybatisSqlInjection.java:29:55:29:58 | name : String |
7-
| MybatisSqlInjection.java:29:55:29:58 | name : String | MybatisSqlInjectionService.java:21:25:21:35 | name : String |
8-
| MybatisSqlInjection.java:34:19:34:40 | test : Test | MybatisSqlInjection.java:35:35:35:38 | test : Test |
9-
| MybatisSqlInjection.java:35:35:35:38 | test : Test | MybatisSqlInjectionService.java:26:19:26:27 | test : Test |
10-
| MybatisSqlInjection.java:39:19:39:40 | test : Test | MybatisSqlInjection.java:40:35:40:38 | test : Test |
11-
| MybatisSqlInjection.java:40:35:40:38 | test : Test | MybatisSqlInjectionService.java:30:19:30:27 | test : Test |
2+
| MybatisSqlInjection.java:17:25:17:35 | name : String | MybatisSqlInjection.java:18:55:18:58 | name : String |
3+
| MybatisSqlInjection.java:18:55:18:58 | name : String | MybatisSqlInjectionService.java:11:25:11:35 | name : String |
4+
| MybatisSqlInjection.java:23:25:23:35 | name : String | MybatisSqlInjection.java:24:55:24:58 | name : String |
5+
| MybatisSqlInjection.java:24:55:24:58 | name : String | MybatisSqlInjectionService.java:16:25:16:35 | name : String |
6+
| MybatisSqlInjection.java:29:25:29:49 | test : Test | MybatisSqlInjection.java:30:55:30:58 | test : Test |
7+
| MybatisSqlInjection.java:30:55:30:58 | test : Test | MybatisSqlInjectionService.java:21:25:21:33 | test : Test |
8+
| MybatisSqlInjection.java:35:19:35:40 | test : Test | MybatisSqlInjection.java:36:35:36:38 | test : Test |
9+
| MybatisSqlInjection.java:36:35:36:38 | test : Test | MybatisSqlInjectionService.java:26:19:26:27 | test : Test |
10+
| MybatisSqlInjection.java:40:19:40:40 | test : Test | MybatisSqlInjection.java:41:35:41:38 | test : Test |
11+
| MybatisSqlInjection.java:41:35:41:38 | test : Test | MybatisSqlInjectionService.java:30:19:30:27 | test : Test |
1212
| MybatisSqlInjectionService.java:11:25:11:35 | name : String | MybatisSqlInjectionService.java:12:47:12:50 | name |
1313
| MybatisSqlInjectionService.java:16:25:16:35 | name : String | MybatisSqlInjectionService.java:17:47:17:50 | name |
14-
| MybatisSqlInjectionService.java:21:25:21:35 | name : String | MybatisSqlInjectionService.java:22:47:22:50 | name |
14+
| MybatisSqlInjectionService.java:21:25:21:33 | test : Test | MybatisSqlInjectionService.java:22:47:22:50 | test |
1515
| MybatisSqlInjectionService.java:26:19:26:27 | test : Test | MybatisSqlInjectionService.java:27:27:27:30 | test |
1616
| MybatisSqlInjectionService.java:30:19:30:27 | test : Test | MybatisSqlInjectionService.java:31:27:31:30 | test |
1717
nodes
18-
| MybatisSqlInjection.java:16:25:16:35 | name : String | semmle.label | name : String |
19-
| MybatisSqlInjection.java:17:55:17:58 | name : String | semmle.label | name : String |
20-
| MybatisSqlInjection.java:22:25:22:35 | name : String | semmle.label | name : String |
21-
| MybatisSqlInjection.java:23:55:23:58 | name : String | semmle.label | name : String |
22-
| MybatisSqlInjection.java:28:25:28:35 | name : String | semmle.label | name : String |
23-
| MybatisSqlInjection.java:29:55:29:58 | name : String | semmle.label | name : String |
24-
| MybatisSqlInjection.java:34:19:34:40 | test : Test | semmle.label | test : Test |
25-
| MybatisSqlInjection.java:35:35:35:38 | test : Test | semmle.label | test : Test |
26-
| MybatisSqlInjection.java:39:19:39:40 | test : Test | semmle.label | test : Test |
27-
| MybatisSqlInjection.java:40:35:40:38 | test : Test | semmle.label | test : Test |
18+
| MybatisSqlInjection.java:17:25:17:35 | name : String | semmle.label | name : String |
19+
| MybatisSqlInjection.java:18:55:18:58 | name : String | semmle.label | name : String |
20+
| MybatisSqlInjection.java:23:25:23:35 | name : String | semmle.label | name : String |
21+
| MybatisSqlInjection.java:24:55:24:58 | name : String | semmle.label | name : String |
22+
| MybatisSqlInjection.java:29:25:29:49 | test : Test | semmle.label | test : Test |
23+
| MybatisSqlInjection.java:30:55:30:58 | test : Test | semmle.label | test : Test |
24+
| MybatisSqlInjection.java:35:19:35:40 | test : Test | semmle.label | test : Test |
25+
| MybatisSqlInjection.java:36:35:36:38 | test : Test | semmle.label | test : Test |
26+
| MybatisSqlInjection.java:40:19:40:40 | test : Test | semmle.label | test : Test |
27+
| MybatisSqlInjection.java:41:35:41:38 | test : Test | semmle.label | test : Test |
2828
| MybatisSqlInjectionService.java:11:25:11:35 | name : String | semmle.label | name : String |
2929
| MybatisSqlInjectionService.java:12:47:12:50 | name | semmle.label | name |
3030
| MybatisSqlInjectionService.java:16:25:16:35 | name : String | semmle.label | name : String |
3131
| MybatisSqlInjectionService.java:17:47:17:50 | name | semmle.label | name |
32-
| MybatisSqlInjectionService.java:21:25:21:35 | name : String | semmle.label | name : String |
33-
| MybatisSqlInjectionService.java:22:47:22:50 | name | semmle.label | name |
32+
| MybatisSqlInjectionService.java:21:25:21:33 | test : Test | semmle.label | test : Test |
33+
| MybatisSqlInjectionService.java:22:47:22:50 | test | semmle.label | test |
3434
| MybatisSqlInjectionService.java:26:19:26:27 | test : Test | semmle.label | test : Test |
3535
| MybatisSqlInjectionService.java:27:27:27:30 | test | semmle.label | test |
3636
| MybatisSqlInjectionService.java:30:19:30:27 | test : Test | semmle.label | test : Test |
3737
| MybatisSqlInjectionService.java:31:27:31:30 | test | semmle.label | test |
3838
#select
39-
| MybatisSqlInjectionService.java:12:47:12:50 | name | MybatisSqlInjection.java:16:25:16:35 | name : String | MybatisSqlInjectionService.java:12:47:12:50 | name | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:16:25:16:35 | name | this user input |
40-
| MybatisSqlInjectionService.java:17:47:17:50 | name | MybatisSqlInjection.java:22:25:22:35 | name : String | MybatisSqlInjectionService.java:17:47:17:50 | name | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:22:25:22:35 | name | this user input |
41-
| MybatisSqlInjectionService.java:22:47:22:50 | name | MybatisSqlInjection.java:28:25:28:35 | name : String | MybatisSqlInjectionService.java:22:47:22:50 | name | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:28:25:28:35 | name | this user input |
42-
| MybatisSqlInjectionService.java:27:27:27:30 | test | MybatisSqlInjection.java:34:19:34:40 | test : Test | MybatisSqlInjectionService.java:27:27:27:30 | test | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:34:19:34:40 | test | this user input |
43-
| MybatisSqlInjectionService.java:31:27:31:30 | test | MybatisSqlInjection.java:39:19:39:40 | test : Test | MybatisSqlInjectionService.java:31:27:31:30 | test | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:39:19:39:40 | test | this user input |
39+
| MybatisSqlInjectionService.java:12:47:12:50 | name | MybatisSqlInjection.java:17:25:17:35 | name : String | MybatisSqlInjectionService.java:12:47:12:50 | name | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:17:25:17:35 | name | this user input |
40+
| MybatisSqlInjectionService.java:17:47:17:50 | name | MybatisSqlInjection.java:23:25:23:35 | name : String | MybatisSqlInjectionService.java:17:47:17:50 | name | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:23:25:23:35 | name | this user input |
41+
| MybatisSqlInjectionService.java:22:47:22:50 | test | MybatisSqlInjection.java:29:25:29:49 | test : Test | MybatisSqlInjectionService.java:22:47:22:50 | test | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:29:25:29:49 | test | this user input |
42+
| MybatisSqlInjectionService.java:27:27:27:30 | test | MybatisSqlInjection.java:35:19:35:40 | test : Test | MybatisSqlInjectionService.java:27:27:27:30 | test | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:35:19:35:40 | test | this user input |
43+
| MybatisSqlInjectionService.java:31:27:31:30 | test | MybatisSqlInjection.java:40:19:40:40 | test : Test | MybatisSqlInjectionService.java:31:27:31:30 | test | MyBatis Mapper XML sql injection might include code from $@. | MybatisSqlInjection.java:40:19:40:40 | test | this user input |

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import org.springframework.stereotype.Controller;
44
import org.springframework.web.bind.annotation.GetMapping;
55
import org.springframework.web.bind.annotation.RequestBody;
6+
import org.springframework.web.bind.annotation.ModelAttribute;
67
import org.springframework.web.bind.annotation.RequestMapping;
78
import org.springframework.web.bind.annotation.RequestMethod;
89

@@ -25,8 +26,8 @@ public List<Test> bad2(String name) {
2526
}
2627

2728
@GetMapping(value = "bad3")
28-
public List<Test> bad3(String name) {
29-
List<Test> result = mybatisSqlInjectionService.bad3(name);
29+
public List<Test> bad3(@ModelAttribute Test test) {
30+
List<Test> result = mybatisSqlInjectionService.bad3(test);
3031
return result;
3132
}
3233

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ public List<Test> bad2(String name) {
1818
return result;
1919
}
2020

21-
public List<Test> bad3(String name) {
22-
List<Test> result = sqlInjectionMapper.bad3(name);
21+
public List<Test> bad3(Test test) {
22+
List<Test> result = sqlInjectionMapper.bad3(test);
2323
return result;
2424
}
2525

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import java.util.List;
22
import org.apache.ibatis.annotations.Mapper;
3+
import org.apache.ibatis.annotations.Param;
34
import org.springframework.stereotype.Repository;
45

56
@Mapper
@@ -8,11 +9,11 @@ public interface SqlInjectionMapper {
89

910
List<Test> bad1(String name);
1011

11-
List<Test> bad2(String name);
12+
List<Test> bad2(@Param("orderby") String name);
1213

13-
List<Test> bad3(String name);
14+
List<Test> bad3(Test test);
1415

15-
void bad4(Test test);
16+
void bad4(@Param("test") Test test);
1617

1718
void bad5(Test test);
1819

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111

1212
<sql id="Update_By_Example_Where_Clause">
1313
<where>
14-
<if test="name != null">
15-
and name = ${name}
14+
<if test="test.name != null">
15+
and name = ${test.name,jdbcType=VARCHAR}
1616
</if>
17-
<if test="id != null">
18-
and id = #{id}
17+
<if test="test.id != null">
18+
and id = #{test.id}
1919
</if>
2020
</where>
2121
</sql>
@@ -24,19 +24,19 @@
2424
select id,name from test where name like '%${name}%'
2525
</select>
2626

27-
<select id="bad2" parameterType="java.lang.String" resultMap="BaseResultMap">
28-
select id,name from test order by ${name} desc
27+
<select id="bad2" resultMap="BaseResultMap">
28+
select id,name from test order by ${orderby,jdbcType=VARCHAR} desc
2929
</select>
3030

3131
<select id="bad3" parameterType="java.lang.String" resultMap="BaseResultMap">
3232
select id,name from test where name in ${name}
3333
</select>
3434

35-
<update id="bad4" parameterType="Test">
35+
<update id="bad4" parameterType="com.example.demo.entity.Test">
3636
update test
3737
<set>
38-
<if test="pass != null">
39-
pass = #{pass},
38+
<if test="test.pass != null">
39+
pass = #{test.pass},
4040
</if>
4141
</set>
4242
<if test="_parameter != null">

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

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

‎java/ql/test/stubs/springframework-5.3.8/org/springframework/web/bind/annotation/ModelAttribute.java

Lines changed: 21 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)