hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 10:15:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11578 from retanoj/MybatisSqli

Browse Source 
Java: Add MyBatis Sql Injection no @Param case
This commit is contained in:
Chris Smowton
2022-12-08 13:53:44 +00:00
committed by GitHub
parent 280bb6864f 0d2474bd55
commit 85ee4e6ca1
6 changed files with 68 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll
View File

@@ -86,7 +86,7 @@ bindingset[unsafeExpression]
predicate isMybatisCollectionTypeSqlInjection(
DataFlow::Node node, MethodAccess ma, string unsafeExpression
) {
not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
not unsafeExpression.regexpMatch("\\$\\{\\s*" + getAMybatisConfigurationVariableKey() + "\\s*\\}") and
// The parameter type of the MyBatis method parameter is Map or List or Array.
// SQL injection vulnerability caused by improper use of this parameter.
// e.g.
@@ -120,7 +120,7 @@ bindingset[unsafeExpression]
predicate isMybatisXmlOrAnnotationSqlInjection(
DataFlow::Node node, MethodAccess ma, string unsafeExpression
) {
not unsafeExpression.regexpMatch("\\$\\{" + getAMybatisConfigurationVariableKey() + "\\}") and
not unsafeExpression.regexpMatch("\\$\\{\\s*" + getAMybatisConfigurationVariableKey() + "\\s*\\}") and
(
// The method parameters use `@Param` annotation. Due to improper use of this parameter, SQL injection vulnerabilities are caused.
// e.g.
@@ -128,11 +128,15 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
// ```java
// @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR})
// void test(@Param("orderby") String name);
//
// @Select(select id,name from test where name = ${ user . name })
// void test(@Param("user") User u);
// ```
exists(Annotation annotation |
unsafeExpression
.matches("${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
"%}") and
.regexpMatch("\\$\\{\\s*" +
annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() +
"\\b[^}]*\\}") and
annotation.getType() instanceof TypeParam and
ma.getAnArgument() = node.asExpr() and
annotation.getTarget() =
@@ -140,6 +144,7 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
)
or
// MyBatis default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n].
// When compiled with '-parameters' compiler option, the parameter can be reflected in SQL statement as named in method signature.
// e.g.
//
// ```java
@@ -149,9 +154,12 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
exists(int i |
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
(
unsafeExpression.matches("${param" + (i + 1) + "%}")
unsafeExpression.regexpMatch("\\$\\{\\s*param" + (i + 1) + "\\b[^}]*\\}")
or
unsafeExpression.matches("${arg" + i + "%}")
unsafeExpression.regexpMatch("\\$\\{\\s*arg" + i + "\\b[^}]*\\}")
or
unsafeExpression
.regexpMatch("\\$\\{\\s*" + ma.getMethod().getParameter(i).getName() + "\\b[^}]*\\}")
) and
ma.getArgument(i) = node.asExpr()
)
@@ -166,7 +174,7 @@ predicate isMybatisXmlOrAnnotationSqlInjection(
exists(int i, RefType t |
not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and
ma.getMethod().getParameterType(i).getName() = t.getName() and
unsafeExpression.matches("${" + t.getAField().getName() + "%}") and
unsafeExpression.regexpMatch("\\$\\{\\s*" + t.getAField().getName() + "\\b[^}]*\\}") and
ma.getArgument(i) = node.asExpr()
)
or

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

@@ -1,30 +1,45 @@
edges
| MybatisSqlInjection.java:62:19:62:43 | name : String | MybatisSqlInjection.java:63:35:63:38 | name : String |
| MybatisSqlInjection.java:63:35:63:38 | name : String | MybatisSqlInjectionService.java:48:19:48:29 | name : String |
| MybatisSqlInjection.java:94:20:94:44 | name : String | MybatisSqlInjection.java:95:36:95:39 | name : String |
| MybatisSqlInjection.java:95:36:95:39 | name : String | MybatisSqlInjectionService.java:76:20:76:30 | name : String |
| MybatisSqlInjection.java:99:20:99:43 | age : String | MybatisSqlInjection.java:100:36:100:38 | age : String |
| MybatisSqlInjection.java:100:36:100:38 | age : String | MybatisSqlInjectionService.java:80:20:80:29 | age : String |
| MybatisSqlInjection.java:67:46:67:70 | name : String | MybatisSqlInjection.java:68:40:68:43 | name : String |
| MybatisSqlInjection.java:68:40:68:43 | name : String | MybatisSqlInjectionService.java:54:32:54:42 | name : String |
| MybatisSqlInjection.java:99:20:99:44 | name : String | MybatisSqlInjection.java:100:36:100:39 | name : String |
| MybatisSqlInjection.java:100:36:100:39 | name : String | MybatisSqlInjectionService.java:80:20:80:30 | name : String |
| MybatisSqlInjection.java:104:20:104:43 | age : String | MybatisSqlInjection.java:105:36:105:38 | age : String |
| MybatisSqlInjection.java:105:36:105:38 | age : String | MybatisSqlInjectionService.java:84:20:84:29 | age : String |
| MybatisSqlInjection.java:109:46:109:70 | name : String | MybatisSqlInjection.java:110:40:110:43 | name : String |
| MybatisSqlInjection.java:110:40:110:43 | name : String | MybatisSqlInjectionService.java:88:32:88:42 | name : String |
| MybatisSqlInjectionService.java:48:19:48:29 | name : String | MybatisSqlInjectionService.java:50:23:50:26 | name : String |
| MybatisSqlInjectionService.java:50:3:50:9 | hashMap [post update] [<map.value>] : String | MybatisSqlInjectionService.java:51:27:51:33 | hashMap |
| MybatisSqlInjectionService.java:50:23:50:26 | name : String | MybatisSqlInjectionService.java:50:3:50:9 | hashMap [post update] [<map.value>] : String |
| MybatisSqlInjectionService.java:76:20:76:30 | name : String | MybatisSqlInjectionService.java:77:28:77:31 | name |
| MybatisSqlInjectionService.java:80:20:80:29 | age : String | MybatisSqlInjectionService.java:81:28:81:30 | age |
| MybatisSqlInjectionService.java:54:32:54:42 | name : String | MybatisSqlInjectionService.java:55:32:55:35 | name |
| MybatisSqlInjectionService.java:80:20:80:30 | name : String | MybatisSqlInjectionService.java:81:28:81:31 | name |
| MybatisSqlInjectionService.java:84:20:84:29 | age : String | MybatisSqlInjectionService.java:85:28:85:30 | age |
| MybatisSqlInjectionService.java:88:32:88:42 | name : String | MybatisSqlInjectionService.java:89:32:89:35 | name |
nodes
| MybatisSqlInjection.java:62:19:62:43 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:63:35:63:38 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:94:20:94:44 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:95:36:95:39 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:99:20:99:43 | age : String | semmle.label | age : String |
| MybatisSqlInjection.java:100:36:100:38 | age : String | semmle.label | age : String |
| MybatisSqlInjection.java:67:46:67:70 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:68:40:68:43 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:99:20:99:44 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:100:36:100:39 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:104:20:104:43 | age : String | semmle.label | age : String |
| MybatisSqlInjection.java:105:36:105:38 | age : String | semmle.label | age : String |
| MybatisSqlInjection.java:109:46:109:70 | name : String | semmle.label | name : String |
| MybatisSqlInjection.java:110:40:110:43 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:48:19:48:29 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:50:3:50:9 | hashMap [post update] [<map.value>] : String | semmle.label | hashMap [post update] [<map.value>] : String |
| MybatisSqlInjectionService.java:50:23:50:26 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:51:27:51:33 | hashMap | semmle.label | hashMap |
| MybatisSqlInjectionService.java:76:20:76:30 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:77:28:77:31 | name | semmle.label | name |
| MybatisSqlInjectionService.java:80:20:80:29 | age : String | semmle.label | age : String |
| MybatisSqlInjectionService.java:81:28:81:30 | age | semmle.label | age |
| MybatisSqlInjectionService.java:54:32:54:42 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:55:32:55:35 | name | semmle.label | name |
| MybatisSqlInjectionService.java:80:20:80:30 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:81:28:81:31 | name | semmle.label | name |
| MybatisSqlInjectionService.java:84:20:84:29 | age : String | semmle.label | age : String |
| MybatisSqlInjectionService.java:85:28:85:30 | age | semmle.label | age |
| MybatisSqlInjectionService.java:88:32:88:42 | name : String | semmle.label | name : String |
| MybatisSqlInjectionService.java:89:32:89:35 | name | semmle.label | name |
subpaths
#select
| 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 |
| MybatisSqlInjectionService.java:55:32:55:35 | name | MybatisSqlInjection.java:67:46:67:70 | name : String | MybatisSqlInjectionService.java:55:32:55:35 | name | MyBatis annotation SQL injection might include code from $@ to $@. | MybatisSqlInjection.java:67:46:67:70 | name | this user input | SqlInjectionMapper.java:36:2:36:72 | Select | this SQL operation |

10
java/ql/test/experimental/query-tests/security/CWE-089/src/main/MybatisSqlInjection.java
View File

@@ -63,6 +63,11 @@ public class MybatisSqlInjection {
mybatisSqlInjectionService.bad9(name);
}
@GetMapping(value = "msi10")
public void bad10(@RequestParam Integer id, @RequestParam String name) {
mybatisSqlInjectionService.bad10(id, name);
}
@GetMapping(value = "good1")
public List<Test> good1(Integer id) {
List<Test> result = mybatisSqlInjectionService.good1(id);
@@ -99,4 +104,9 @@ public class MybatisSqlInjection {
public void good3(@RequestParam String age) {
mybatisSqlInjectionService.good3(age);
}
@GetMapping(value = "good4")
public void good4(@RequestParam Integer id, @RequestParam String name) {
mybatisSqlInjectionService.good4(id, name);
}
}

8
java/ql/test/experimental/query-tests/security/CWE-089/src/main/MybatisSqlInjectionService.java
View File

@@ -51,6 +51,10 @@ public class MybatisSqlInjectionService {
sqlInjectionMapper.bad9(hashMap);
}
public void bad10(Integer id, String name) {
sqlInjectionMapper.bad10(id, name);
}
public List<Test> good1(Integer id) {
List<Test> result = sqlInjectionMapper.good1(id);
return result;
@@ -80,4 +84,8 @@ public class MybatisSqlInjectionService {
public void good3(String age){
sqlInjectionMapper.good3(age);
}
public void good4(Integer id, String name) {
sqlInjectionMapper.good4(id, name);
}
}

5
java/ql/test/experimental/query-tests/security/CWE-089/src/main/SqlInjectionMapper.java
View File

@@ -33,6 +33,9 @@ public interface SqlInjectionMapper {
@Select({"select * from test", "where id = ${name}"})
public Test bad9(HashMap<String, Object> map);
@Select({"select * from test where id = #{id} and name = '${ name }'"})
String bad10(Integer id, String name);
List<Test> good1(Integer id);
//using providers
@@ -66,4 +69,6 @@ public interface SqlInjectionMapper {
@Select("select * from user_info where age = #{age}")
String good3(@Param("age") String age);
@Select({"select * from test where id = #{id} and name = #{name}"})
String good4(Integer id, String name);
}

2
java/ql/test/experimental/query-tests/security/CWE-089/src/main/SqlInjectionMapper.xml
View File

@@ -12,7 +12,7 @@
<sql id="Update_By_Example_Where_Clause">
<where>
<if test="test.name != null">
and name = ${test.name,jdbcType=VARCHAR}
and name = ${ test . name , jdbcType = VARCHAR }
</if>
<if test="test.id != null">
and id = #{test.id}