Skip to content

Commit db04a0d

Browse files
haby0haby0
committed
New model: SQL injection in MyBatis annotations
1 parent 04a3f76 commit db04a0d

16 files changed

+549
-114
lines changed

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,119 @@ class IbatisConfigurationGetVariablesMethod extends Method {
4040
getNumberOfParameters() = 0
4141
}
4242
}
43+
44+
/**
45+
* An annotation type that identifies Ibatis select.
46+
*/
47+
private class IbatisSelectAnnotationType extends AnnotationType {
48+
IbatisSelectAnnotationType() {
49+
this.hasQualifiedName("org.apache.ibatis.annotations", "Select") or
50+
this.getAnAnnotation().getType() instanceof IbatisSelectAnnotationType
51+
}
52+
}
53+
54+
/**
55+
* An annotation type that identifies Ibatis delete.
56+
*/
57+
private class IbatisDeleteAnnotationType extends AnnotationType {
58+
IbatisDeleteAnnotationType() {
59+
this.hasQualifiedName("org.apache.ibatis.annotations", "Delete") or
60+
this.getAnAnnotation().getType() instanceof IbatisDeleteAnnotationType
61+
}
62+
}
63+
64+
/**
65+
* An annotation type that identifies Ibatis insert.
66+
*/
67+
private class IbatisInsertAnnotationType extends AnnotationType {
68+
IbatisInsertAnnotationType() {
69+
this.hasQualifiedName("org.apache.ibatis.annotations", "Insert") or
70+
this.getAnAnnotation().getType() instanceof IbatisInsertAnnotationType
71+
}
72+
}
73+
74+
/**
75+
* An annotation type that identifies Ibatis update.
76+
*/
77+
private class IbatisUpdateAnnotationType extends AnnotationType {
78+
IbatisUpdateAnnotationType() {
79+
this.hasQualifiedName("org.apache.ibatis.annotations", "Update") or
80+
this.getAnAnnotation().getType() instanceof IbatisUpdateAnnotationType
81+
}
82+
}
83+
84+
/**
85+
* Ibatis sql operation annotation.
86+
*/
87+
abstract class IbatisSqlOperationAnnotation extends Annotation {
88+
abstract string getSqlValue();
89+
}
90+
91+
/**
92+
* A `@org.apache.ibatis.annotations.Select` annotation.
93+
*/
94+
private class IbatisSelectAnnotation extends IbatisSqlOperationAnnotation {
95+
IbatisSelectAnnotation() { this.getType() instanceof IbatisSelectAnnotationType }
96+
97+
string getSelectValue() {
98+
result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or
99+
result =
100+
this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue()
101+
}
102+
103+
override string getSqlValue() { result = getSelectValue() }
104+
}
105+
106+
/**
107+
* A `@org.apache.ibatis.annotations.Delete` annotation.
108+
*/
109+
private class IbatisDeleteAnnotation extends IbatisSqlOperationAnnotation {
110+
IbatisDeleteAnnotation() { this.getType() instanceof IbatisDeleteAnnotationType }
111+
112+
string getDeleteValue() {
113+
result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or
114+
result =
115+
this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue()
116+
}
117+
118+
override string getSqlValue() { result = getDeleteValue() }
119+
}
120+
121+
/**
122+
* A `@org.apache.ibatis.annotations.Insert` annotation.
123+
*/
124+
private class IbatisInsertAnnotation extends IbatisSqlOperationAnnotation {
125+
IbatisInsertAnnotation() { this.getType() instanceof IbatisInsertAnnotationType }
126+
127+
string getInsertValue() {
128+
result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or
129+
result =
130+
this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue()
131+
}
132+
133+
override string getSqlValue() { result = getInsertValue() }
134+
}
135+
136+
/**
137+
* A `@org.apache.ibatis.annotations.Update` annotation.
138+
*/
139+
private class IbatisUpdateAnnotation extends IbatisSqlOperationAnnotation {
140+
IbatisUpdateAnnotation() { this.getType() instanceof IbatisUpdateAnnotationType }
141+
142+
string getUpdateValue() {
143+
result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or
144+
result =
145+
this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue()
146+
}
147+
148+
override string getSqlValue() { result = getUpdateValue() }
149+
}
150+
151+
// Mybatis uses sql operation to annotate the method of interacting with the database.
152+
class MybatisSqlOperationAnnotationMethod extends Method {
153+
MybatisSqlOperationAnnotationMethod() {
154+
exists(IbatisSqlOperationAnnotation isoa |
155+
this.getAnAnnotation() = isoa
156+
)
157+
}
158+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import org.apache.ibatis.annotations.Select;
2+
3+
public interface MyBatisAnnotationSqlInjection {
4+
5+
@Select("select * from test where name = ${name}")
6+
public Test bad1(String name);
7+
8+
@Select("select * from test where name = #{name}")
9+
public Test good1(String name);
10+
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
<!DOCTYPE qhelp PUBLIC
3+
"-//Semmle//qhelp//EN"
4+
"qhelp.dtd">
5+
<qhelp>
6+
<overview>
7+
<p>MyBatis operates the database by using @Select, @Insert, etc. annotations in the method, and can use the $ character
8+
to construct dynamic SQL statements. Attackers can modify the meaning of statements or execute arbitrary SQL commands.</p>
9+
</overview>
10+
11+
<<recommendation>
12+
<p>
13+
When writing MyBatis mapping statements, try to use the format "#{xxx}". If you have to use parameters
14+
such as "${xxx}", you must manually filter to prevent SQL injection attacks.
15+
</p>
16+
</recommendation>
17+
18+
<example>
19+
<p>
20+
The following examples show the bad situation and the good situation respectively. The <code>bad1</code> method uses <code>$(name)</code>
21+
in the <code>@Select</code> annotation to dynamically splice SQL statements, and there is a SQL injection vulnerability.
22+
The good1 method uses the <code>#{name}</code> method in the <code>@Select</code> annotation to splice SQL statements,
23+
and the MyBatis framework will handle the dangerous characters entered by the user, And did not cause SQL injection vulnerabilities.
24+
</p>
25+
<sample src="MyBatisAnnotationSqlInjection.java" />
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/MyBatisAnnotationSqlInjection.ql

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* @name MyBatis annotation 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 MyBatisAnnotationSqlInjectionLib
17+
import semmle.code.java.security.SanitizerGuard
18+
import semmle.code.java.dataflow.FlowSources
19+
20+
private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::Configuration {
21+
MyBatisAnnotationSqlInjectionConfiguration() { this = "MyBatis annotation sql injection" }
22+
23+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
24+
25+
override predicate isSink(DataFlow::Node sink) {
26+
sink instanceof MyBatisAnnotationMethodCallAnArgument
27+
}
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+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
36+
guard instanceof ContainsSanitizer or guard instanceof EqualsSanitizer
37+
}
38+
39+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
40+
exists(MethodAccess ma |
41+
ma.getMethod().getDeclaringType() instanceof MapType and
42+
ma.getMethod().getName() = "get" and
43+
ma.getQualifier() = node1.asExpr() and
44+
ma = node2.asExpr()
45+
)
46+
or
47+
exists(MethodAccess ma |
48+
ma.getMethod().getDeclaringType() instanceof TypeObject and
49+
ma.getMethod().getName() = "toString" and
50+
ma.getQualifier() = node1.asExpr() and
51+
ma = node2.asExpr()
52+
)
53+
}
54+
}
55+
56+
from
57+
MyBatisAnnotationSqlInjectionConfiguration cfg, DataFlow::PathNode source,
58+
DataFlow::PathNode sink, IbatisSqlOperationAnnotation isoa
59+
where
60+
cfg.hasFlowPath(source, sink) and
61+
isMybatisAnnotationSqlInjection(sink.getNode(), isoa)
62+
select sink.getNode(), source, sink,
63+
"MyBatis annotation sql injection might include code from $@ to $@.", source.getNode(),
64+
"this user input", isoa, "this sql operation"

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

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import java
2+
import MyBatisCommonLib
3+
import semmle.code.xml.MyBatisMapperXML
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.frameworks.Properties
6+
7+
/** A sink for MyBatis annotation method call an argument. */
8+
class MyBatisAnnotationMethodCallAnArgument extends DataFlow::Node {
9+
MyBatisAnnotationMethodCallAnArgument() {
10+
exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma | ma.getMethod() = msoam |
11+
ma.getAnArgument() = this.asExpr()
12+
)
13+
}
14+
}
15+
16+
/** Get the #{...} or ${...} parameters in the Mybatis annotation value. */
17+
private string getAnMybatiAnnotationSetValue(IbatisSqlOperationAnnotation isoa) {
18+
result = isoa.getSqlValue().trim().regexpFind("(#|\\$)(\\{([^\\}]*\\}))", _, _)
19+
}
20+
21+
predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperationAnnotation isoa) {
22+
// MyBatis uses an annotation method to perform SQL operations. This method has only one parameter and
23+
// the parameter is not annotated with `@Param`. Improper use of this parameter has a SQL injection vulnerability.
24+
// e.g.
25+
//
26+
// ```java
27+
// @Select(select id,name from test where name like '%${value}%')
28+
// Test test(String name);
29+
// ```
30+
exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, string res |
31+
msoam = ma.getMethod()
32+
|
33+
msoam.getAnAnnotation() = isoa and
34+
res = getAnMybatiAnnotationSetValue(isoa) and
35+
msoam.getNumberOfParameters() = 1 and
36+
not ma.getMethod().getAParameter().hasAnnotation() and
37+
res.matches("%${%}") and
38+
not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and
39+
ma.getAnArgument() = node.asExpr()
40+
)
41+
or
42+
// MyBatis uses an annotation method to perform SQL operations. The parameter type of the method parameter
43+
// is Map or List or Array. SQL injection vulnerability caused by improper use of this parameter.
44+
// e.g.
45+
//
46+
// ```java
47+
// @Select(select id,name from test where name like '%${value}%')
48+
// Test test(Map map);
49+
// ```
50+
exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res |
51+
msoam = ma.getMethod()
52+
|
53+
msoam.getAnAnnotation() = isoa and
54+
not ma.getMethod().getParameter(i).hasAnnotation() and
55+
(
56+
ma.getMethod().getParameterType(i) instanceof MapType or
57+
ma.getMethod().getParameterType(i) instanceof ListType or
58+
ma.getMethod().getParameterType(i) instanceof Array
59+
) and
60+
res = getAnMybatiAnnotationSetValue(isoa) and
61+
res.matches("%${%}") and
62+
not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and
63+
ma.getArgument(i) = node.asExpr()
64+
)
65+
or
66+
// MyBatis uses annotation methods to perform SQL operations, and SQL injection vulnerabilities caused by
67+
// improper use of instance class fields.
68+
// e.g.
69+
//
70+
// ```java
71+
// @Select(select id,name from test order by ${name,jdbcType=VARCHAR})
72+
// void test(Test test);
73+
// ```
74+
exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Class c |
75+
msoam = ma.getMethod()
76+
|
77+
msoam.getAnAnnotation() = isoa and
78+
not ma.getMethod().getParameter(i).hasAnnotation() and
79+
ma.getMethod().getParameterType(i).getName() = c.getName() and
80+
getAnMybatiAnnotationSetValue(isoa).matches("%${" + c.getAField().getName() + "%}") and
81+
ma.getArgument(i) = node.asExpr()
82+
)
83+
or
84+
// MyBatis uses annotations to perform SQL operations. The method parameters use `@Param` annotation.
85+
// Due to improper use of this parameter, SQL injection vulnerabilities are caused.
86+
// e.g.
87+
//
88+
// ```java
89+
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
90+
// void test(@Param("orderby") String name);
91+
// ```
92+
exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Annotation annotation |
93+
msoam = ma.getMethod()
94+
|
95+
msoam.getAnAnnotation() = isoa and
96+
ma.getMethod().getParameter(i).hasAnnotation() and
97+
ma.getMethod().getParameter(i).getAnAnnotation() = annotation and
98+
annotation.getType() instanceof TypeParam and
99+
getAnMybatiAnnotationSetValue(isoa)
100+
.matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
101+
"%}") and
102+
ma.getArgument(i) = node.asExpr()
103+
)
104+
or
105+
// MyBatis Mapper method default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
106+
// e.g.
107+
//
108+
// ```java
109+
// @Select(select id,name from test order by ${arg0,jdbcType=VARCHAR})
110+
// void test(String name);
111+
// ```
112+
exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res |
113+
msoam = ma.getMethod()
114+
|
115+
msoam.getAnAnnotation() = isoa and
116+
not ma.getMethod().getParameter(i).hasAnnotation() and
117+
res = getAnMybatiAnnotationSetValue(isoa) and
118+
(
119+
res.matches("%${param" + (i + 1) + "%}")
120+
or
121+
res.matches("%${arg" + i + "%}")
122+
) and
123+
not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and
124+
ma.getArgument(i) = node.asExpr()
125+
)
126+
}

0 commit comments

Comments
 (0)