Skip to content

Commit 4438f8c

Browse files
haby0haby0
committed
Add MyBatis Mapper Sql Injection
1 parent d3da790 commit 4438f8c

File tree

16 files changed

+589
-0
lines changed

16 files changed

+589
-0
lines changed

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The MyBatis Mapper XML file allows the use of the $ character to construct dynamic SQL statements.
7+
Attackers can modify the meaning of statements or execute arbitrary SQL commands.</p>
8+
</overview>
9+
10+
<<recommendation>
11+
<p>
12+
When writing MyBatis mapping statements, try to use the format "#{xxx}". If you have to use parameters
13+
such as "${xxx}", you must manually filter to prevent SQL injection attacks.
14+
</p>
15+
</recommendation>
16+
17+
<example>
18+
<p>
19+
The following examples show the bad situation and the good situation respectively. In <code>bad1</code>
20+
and <code>bad2</code> and <code>bad3</code> and <code>bad4</code> and <code >bad5</code>, the program
21+
${ xxx} are dynamic SQL statements, these five examples of SQL injection vulnerabilities. In <code>good1</code>,
22+
the program uses the ${xxx} dynamic feature SQL statement, but there are subtle restrictions on the data,
23+
and there is no SQL injection vulnerability.
24+
</p>
25+
<sample src="MyBatisMapperXmlSqlInjection.xml" />
26+
</example>
27+
28+
<references>
29+
<li>
30+
Fortify:
31+
<a href="https://vulncat.fortify.com/en/detail?id=desc.config.java.sql_injection_mybatis_mapper">SQL Injection: MyBatis Mapper</a>.
32+
</li>
33+
</references>
34+
</qhelp>

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/**
2+
* @name MyBatis Mapper xml sql injection
3+
* @description Constructing a dynamic SQL statement with input that comes from an
4+
* untrusted source could allow an attacker to modify the statement's
5+
* meaning or to execute arbitrary SQL commands.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @precision high
9+
* @id java/sql-injection
10+
* @tags security
11+
* external/cwe/cwe-089
12+
*/
13+
14+
import java
15+
import DataFlow::PathGraph
16+
import MyBatisMapperXmlSqlInjectionLib
17+
import semmle.code.java.dataflow.FlowSources
18+
19+
/**
20+
* A taint-tracking configuration for tracking untrusted user input used by the Mybatis mapper xml file dynamic splicing sql use.
21+
*/
22+
private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::Configuration {
23+
MyBatisMapperXmlSqlInjectionConfiguration() { this = "MyBatis mapper xml sql injection" }
24+
25+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
26+
27+
override predicate isSink(DataFlow::Node sink) { sink instanceof MyBatisMapperXmlSqlInjectionSink }
28+
29+
override predicate isSanitizer(DataFlow::Node node) {
30+
node.getType() instanceof PrimitiveType or
31+
node.getType() instanceof BoxedType or
32+
node.getType() instanceof NumberType
33+
}
34+
}
35+
36+
from MyBatisMapperXmlSqlInjectionConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
37+
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"

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?xml version="1.0" encoding="UTF-8" ?>
2+
<!DOCTYPE mapper PUBLIC "-//mybatis.org//DTD Mapper 3.0//EN"
3+
"http://mybatis.org/dtd/mybatis-3-mapper.dtd" >
4+
<mapper namespace="SqlInjectionMapper">
5+
6+
<resultMap id="BaseResultMap" type="Test">
7+
<id column="id" jdbcType="INTEGER" property="id"/>
8+
<result column="name" jdbcType="VARCHAR" property="name"/>
9+
<result column="pass" jdbcType="VARCHAR" property="pass"/>
10+
</resultMap>
11+
12+
<sql id="Update_By_Example_Where_Clause">
13+
<where>
14+
<if test="name != null">
15+
-- bad
16+
and name = ${name}
17+
</if>
18+
<if test="id != null">
19+
and id = #{id}
20+
</if>
21+
</where>
22+
</sql>
23+
24+
<select id="bad1" parameterType="java.lang.String" resultMap="BaseResultMap">
25+
-- bad
26+
select id,name from test where name like '%${name}%'
27+
</select>
28+
29+
<select id="bad2" parameterType="java.lang.String" resultMap="BaseResultMap">
30+
-- bad
31+
select id,name from test order by ${name} desc
32+
</select>
33+
34+
<select id="bad3" parameterType="java.lang.String" resultMap="BaseResultMap">
35+
-- bad
36+
select id,name from test where name in ${name}
37+
</select>
38+
39+
<update id="bad4" parameterType="Test">
40+
update test
41+
<set>
42+
<if test="pass != null">
43+
pass = #{pass},
44+
</if>
45+
</set>
46+
<if test="_parameter != null">
47+
-- bad
48+
<include refid="Update_By_Example_Where_Clause" />
49+
</if>
50+
</update>
51+
52+
<insert id="bad5" parameterType="Test">
53+
insert into test (name, pass)
54+
<trim prefix="values (" suffix=")" suffixOverrides=",">
55+
<if test="name != null">
56+
-- bad
57+
name = ${name},
58+
</if>
59+
<if test="pass != null">
60+
-- bad
61+
pass = ${pass},
62+
</if>
63+
</trim>
64+
</insert>
65+
66+
<select id="good1" parameterType="java.lang.Integer" resultMap="BaseResultMap">
67+
-- good
68+
select id,name from test where id = ${id}
69+
</select>
70+
</mapper>

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import java
2+
import semmle.code.xml.MyBatisMapperXML
3+
import semmle.code.java.dataflow.FlowSources
4+
5+
/** 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
10+
mbmxe.getId() = ma.getMethod().getName() and
11+
ma.getMethod().getDeclaringType() =
12+
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType() and
13+
ma.getAnArgument() = this.asExpr()
14+
)
15+
or
16+
exists(MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess ma |
17+
mbmxe.getInclude().getRefid() = mbms.getId() and
18+
mbms.getAChild*().getTextValue().trim().matches("%${%") and
19+
mbmxe.getId() = ma.getMethod().getName() and
20+
ma.getMethod().getDeclaringType() =
21+
mbmxe.getParent().(MyBatisMapperXMLElement).getNamespaceRefType() and
22+
ma.getAnArgument() = this.asExpr()
23+
)
24+
}
25+
}

‎java/ql/src/semmle/code/xml/MyBatisMapperXML.qll

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import java
2+
3+
/**
4+
* MyBatis Mapper XML file.
5+
*/
6+
class MyBatisMapperXMLFile extends XMLFile {
7+
MyBatisMapperXMLFile() {
8+
count(XMLElement e | e = this.getAChild()) = 1 and
9+
this.getAChild().getName() = "mapper"
10+
}
11+
}
12+
13+
/**
14+
* An XML element in a `MyBatisMapperXMLFile`.
15+
*/
16+
class MyBatisMapperXMLElement extends XMLElement {
17+
MyBatisMapperXMLElement() { this.getFile() instanceof MyBatisMapperXMLFile }
18+
19+
/**
20+
* Gets the value for this element, with leading and trailing whitespace trimmed.
21+
*/
22+
string getValue() { result = allCharactersString().trim() }
23+
24+
/**
25+
* Gets the reference type bound to MyBatis Mapper XML File.
26+
*/
27+
RefType getNamespaceRefType() { result.getQualifiedName() = getAttribute("namespace").getValue() }
28+
}
29+
30+
/**
31+
* An MyBatis Mapper sql operation element.
32+
*/
33+
abstract class MyBatisMapperSqlOperation extends MyBatisMapperXMLElement {
34+
abstract string getId();
35+
36+
/**
37+
* Gets the `<include>` element in a `MyBatisMapperSqlOperation`.
38+
*/
39+
MyBatisMapperInclude getInclude() { result = getAChild*() }
40+
}
41+
42+
/**
43+
* A `<insert>` element in a `MyBatisMapperSqlOperation`.
44+
*/
45+
class MyBatisMapperInsert extends MyBatisMapperSqlOperation {
46+
MyBatisMapperInsert() { getName() = "insert" }
47+
48+
/**
49+
* Gets the value of the `id` attribute of this `<insert>`.
50+
*/
51+
override string getId() { result = getAttribute("id").getValue() }
52+
}
53+
54+
/**
55+
* A `<update>` element in a `MyBatisMapperSqlOperation`.
56+
*/
57+
class MyBatisMapperUpdate extends MyBatisMapperSqlOperation {
58+
MyBatisMapperUpdate() { getName() = "update" }
59+
60+
/**
61+
* Gets the value of the `id` attribute of this `<update>`.
62+
*/
63+
override string getId() { result = getAttribute("id").getValue() }
64+
}
65+
66+
/**
67+
* A `<delete>` element in a `MyBatisMapperSqlOperation`.
68+
*/
69+
class MyBatisMapperDelete extends MyBatisMapperSqlOperation {
70+
MyBatisMapperDelete() { getName() = "delete" }
71+
72+
/**
73+
* Gets the value of the `id` attribute of this `<delete>`.
74+
*/
75+
override string getId() { result = getAttribute("id").getValue() }
76+
}
77+
78+
/**
79+
* A `<select>` element in a `MyBatisMapperSqlOperation`.
80+
*/
81+
class MyBatisMapperSelect extends MyBatisMapperSqlOperation {
82+
MyBatisMapperSelect() { getName() = "select" }
83+
84+
/**
85+
* Gets the value of the `id` attribute of this `<select>`.
86+
*/
87+
override string getId() { result = getAttribute("id").getValue() }
88+
}
89+
90+
/**
91+
* A `<select>` element in a `MyBatisMapperXMLElement`.
92+
*/
93+
class MyBatisMapperSql extends MyBatisMapperXMLElement {
94+
MyBatisMapperSql() { getName() = "sql" }
95+
96+
/**
97+
* Gets the value of the `id` attribute of this `<sql>`.
98+
*/
99+
string getId() { result = getAttribute("id").getValue() }
100+
}
101+
102+
/**
103+
* A `<include>` element in a `MyBatisMapperXMLElement`.
104+
*/
105+
class MyBatisMapperInclude extends MyBatisMapperXMLElement {
106+
MyBatisMapperInclude() { getName() = "include" }
107+
108+
/**
109+
* Gets the value of the `refid` attribute of this `<include>`.
110+
*/
111+
string getRefid() { result = getAttribute("refid").getValue() }
112+
}

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
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 |
12+
| MybatisSqlInjectionService.java:11:25:11:35 | name : String | MybatisSqlInjectionService.java:12:47:12:50 | name |
13+
| 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 |
15+
| MybatisSqlInjectionService.java:26:19:26:27 | test : Test | MybatisSqlInjectionService.java:27:27:27:30 | test |
16+
| MybatisSqlInjectionService.java:30:19:30:27 | test : Test | MybatisSqlInjectionService.java:31:27:31:30 | test |
17+
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 |
28+
| MybatisSqlInjectionService.java:11:25:11:35 | name : String | semmle.label | name : String |
29+
| MybatisSqlInjectionService.java:12:47:12:50 | name | semmle.label | name |
30+
| MybatisSqlInjectionService.java:16:25:16:35 | name : String | semmle.label | name : String |
31+
| 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 |
34+
| MybatisSqlInjectionService.java:26:19:26:27 | test : Test | semmle.label | test : Test |
35+
| MybatisSqlInjectionService.java:27:27:27:30 | test | semmle.label | test |
36+
| MybatisSqlInjectionService.java:30:19:30:27 | test : Test | semmle.label | test : Test |
37+
| MybatisSqlInjectionService.java:31:27:31:30 | test | semmle.label | test |
38+
#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 |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import java.util.List;
2+
import org.springframework.beans.factory.annotation.Autowired;
3+
import org.springframework.stereotype.Controller;
4+
import org.springframework.web.bind.annotation.GetMapping;
5+
import org.springframework.web.bind.annotation.RequestBody;
6+
import org.springframework.web.bind.annotation.RequestMapping;
7+
import org.springframework.web.bind.annotation.RequestMethod;
8+
9+
@Controller
10+
public class MybatisSqlInjection {
11+
12+
@Autowired
13+
private MybatisSqlInjectionService mybatisSqlInjectionService;
14+
15+
@GetMapping(value = "bad1")
16+
public List<Test> bad1(String name) {
17+
List<Test> result = mybatisSqlInjectionService.bad1(name);
18+
return result;
19+
}
20+
21+
@GetMapping(value = "bad2")
22+
public List<Test> bad2(String name) {
23+
List<Test> result = mybatisSqlInjectionService.bad2(name);
24+
return result;
25+
}
26+
27+
@GetMapping(value = "bad3")
28+
public List<Test> bad3(String name) {
29+
List<Test> result = mybatisSqlInjectionService.bad3(name);
30+
return result;
31+
}
32+
33+
@RequestMapping(value = "bad4", method = RequestMethod.POST, produces = "application/json")
34+
public void bad4(@RequestBody Test test) {
35+
mybatisSqlInjectionService.bad4(test);
36+
}
37+
38+
@RequestMapping(value = "bad5", method = RequestMethod.PUT, produces = "application/json")
39+
public void bad5(@RequestBody Test test) {
40+
mybatisSqlInjectionService.bad5(test);
41+
}
42+
43+
@GetMapping(value = "good1")
44+
public List<Test> good1(Integer id) {
45+
List<Test> result = mybatisSqlInjectionService.good1(id);
46+
return result;
47+
}
48+
}

0 commit comments

Comments
 (0)