diff --git a/java/ql/lib/semmle/code/java/frameworks/MyBatis.qll b/java/ql/lib/semmle/code/java/frameworks/MyBatis.qll index 5d18555509a..0056098877a 100644 --- a/java/ql/lib/semmle/code/java/frameworks/MyBatis.qll +++ b/java/ql/lib/semmle/code/java/frameworks/MyBatis.qll @@ -35,9 +35,9 @@ class IbatisConfiguration extends RefType { */ class IbatisConfigurationGetVariablesMethod extends Method { IbatisConfigurationGetVariablesMethod() { - getDeclaringType() instanceof IbatisConfiguration and - hasName("getVariables") and - getNumberOfParameters() = 0 + this.getDeclaringType() instanceof IbatisConfiguration and + this.hasName("getVariables") and + this.getNumberOfParameters() = 0 } } @@ -45,114 +45,62 @@ class IbatisConfigurationGetVariablesMethod extends Method { * An annotation type that identifies Ibatis select. */ private class IbatisSelectAnnotationType extends AnnotationType { - IbatisSelectAnnotationType() { - this.hasQualifiedName("org.apache.ibatis.annotations", "Select") or - this.getAnAnnotation().getType() instanceof IbatisSelectAnnotationType - } + IbatisSelectAnnotationType() { this.hasQualifiedName("org.apache.ibatis.annotations", "Select") } } /** * An annotation type that identifies Ibatis delete. */ private class IbatisDeleteAnnotationType extends AnnotationType { - IbatisDeleteAnnotationType() { - this.hasQualifiedName("org.apache.ibatis.annotations", "Delete") or - this.getAnAnnotation().getType() instanceof IbatisDeleteAnnotationType - } + IbatisDeleteAnnotationType() { this.hasQualifiedName("org.apache.ibatis.annotations", "Delete") } } /** * An annotation type that identifies Ibatis insert. */ private class IbatisInsertAnnotationType extends AnnotationType { - IbatisInsertAnnotationType() { - this.hasQualifiedName("org.apache.ibatis.annotations", "Insert") or - this.getAnAnnotation().getType() instanceof IbatisInsertAnnotationType - } + IbatisInsertAnnotationType() { this.hasQualifiedName("org.apache.ibatis.annotations", "Insert") } } /** * An annotation type that identifies Ibatis update. */ private class IbatisUpdateAnnotationType extends AnnotationType { - IbatisUpdateAnnotationType() { - this.hasQualifiedName("org.apache.ibatis.annotations", "Update") or - this.getAnAnnotation().getType() instanceof IbatisUpdateAnnotationType - } + IbatisUpdateAnnotationType() { this.hasQualifiedName("org.apache.ibatis.annotations", "Update") } } /** * Ibatis sql operation annotation. */ -abstract class IbatisSqlOperationAnnotation extends Annotation { - abstract string getSqlValue(); -} +class IbatisSqlOperationAnnotation extends Annotation { + IbatisSqlOperationAnnotation() { + this.getType() instanceof IbatisSelectAnnotationType or + this.getType() instanceof IbatisDeleteAnnotationType or + this.getType() instanceof IbatisInsertAnnotationType or + this.getType() instanceof IbatisUpdateAnnotationType + } -/** - * A `@org.apache.ibatis.annotations.Select` annotation. - */ -private class IbatisSelectAnnotation extends IbatisSqlOperationAnnotation { - IbatisSelectAnnotation() { this.getType() instanceof IbatisSelectAnnotationType } - - string getSelectValue() { + /** + * Get the SQL statement string. + */ + string getSqlValue() { result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or result = this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue() } - - override string getSqlValue() { result = getSelectValue() } } /** - * A `@org.apache.ibatis.annotations.Delete` annotation. + * Methods annotated with `@org.apache.ibatis.annotations.Select` or `@org.apache.ibatis.annotations.Delete` + * or `@org.apache.ibatis.annotations.Update` or `@org.apache.ibatis.annotations.Insert`. */ -private class IbatisDeleteAnnotation extends IbatisSqlOperationAnnotation { - IbatisDeleteAnnotation() { this.getType() instanceof IbatisDeleteAnnotationType } - - string getDeleteValue() { - result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or - result = - this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue() - } - - override string getSqlValue() { result = getDeleteValue() } -} - -/** - * A `@org.apache.ibatis.annotations.Insert` annotation. - */ -private class IbatisInsertAnnotation extends IbatisSqlOperationAnnotation { - IbatisInsertAnnotation() { this.getType() instanceof IbatisInsertAnnotationType } - - string getInsertValue() { - result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or - result = - this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue() - } - - override string getSqlValue() { result = getInsertValue() } -} - -/** - * A `@org.apache.ibatis.annotations.Update` annotation. - */ -private class IbatisUpdateAnnotation extends IbatisSqlOperationAnnotation { - IbatisUpdateAnnotation() { this.getType() instanceof IbatisUpdateAnnotationType } - - string getUpdateValue() { - result = this.getValue("value").(CompileTimeConstantExpr).getStringValue() or - result = - this.getValue("value").(ArrayInit).getInit(_).(CompileTimeConstantExpr).getStringValue() - } - - override string getSqlValue() { result = getUpdateValue() } -} - -// Mybatis uses sql operation to annotate the method of interacting with the database. -class MybatisSqlOperationAnnotationMethod extends Method { - MybatisSqlOperationAnnotationMethod() { - exists(IbatisSqlOperationAnnotation isoa | - this.getAnAnnotation() = isoa - ) +class MyBatisSqlOperationAnnotationMethod extends Method { + MyBatisSqlOperationAnnotationMethod() { + this.getAnAnnotation() instanceof IbatisSqlOperationAnnotation } } + +/** The interface `org.apache.ibatis.annotations.Param`. */ +class TypeParam extends Interface { + TypeParam() { this.hasQualifiedName("org.apache.ibatis.annotations", "Param") } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.qhelp index 75dace474dc..ae631d55481 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.qhelp @@ -4,23 +4,21 @@ "qhelp.dtd"> -

MyBatis operates the database by using @Select, @Insert, etc. annotations in the method, and can use the $ character -to construct dynamic SQL statements. Attackers can modify the meaning of statements or execute arbitrary SQL commands.

+

MyBatis allows operating the database by annotating a method with the annotations @Select, @Insert, etc. to construct dynamic SQL statements. +If the syntax `${param}` is used in those statements, and `param` is a parameter of the annotated method, attackers can exploit this to tamper with the SQL statements or execute arbitrary SQL commands.

 <

-When writing MyBatis mapping statements, try to use the format "#{xxx}". If you have to use parameters -such as "${xxx}", you must manually filter to prevent SQL injection attacks. +When writing MyBatis mapping statements, try to use the syntax #{xxx}. If the syntax ${xxx} must be used, any parameters included in it should be sanitized to prevent SQL injection attacks.

-The following examples show the bad situation and the good situation respectively. The bad1 method uses $(name) -in the @Select annotation to dynamically splice SQL statements, and there is a SQL injection vulnerability. -The good1 method uses the #{name} method in the @Select annotation to splice SQL statements, -and the MyBatis framework will handle the dangerous characters entered by the user, And did not cause SQL injection vulnerabilities. +The following sample shows a bad and a good example of MyBatis annotations usage. The bad1 method uses $(name) +in the @Select annotation to dynamically build a SQL statement, which causes a SQL injection vulnerability. +The good1 method uses #{name} in the @Select annotation to to dynamically include the parameter in a SQL statement, which allows the MyBatis framework to handle the sanitization, preventing the vulnerability.

diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.ql index e9f7a6c10e6..32aaaa4e5a3 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjection.ql @@ -1,12 +1,12 @@ /** - * @name MyBatis annotation sql injection + * @name SQL injection in MyBatis annotation * @description Constructing a dynamic SQL statement with input that comes from an * untrusted source could allow an attacker to modify the statement's * meaning or to execute arbitrary SQL commands. * @kind path-problem * @problem.severity error * @precision high - * @id java/sql-injection + * @id java/mybatis-annotation-sql-injection * @tags security * external/cwe/cwe-089 */ @@ -14,7 +14,6 @@ import java import DataFlow::PathGraph import MyBatisAnnotationSqlInjectionLib -import semmle.code.java.security.SanitizerGuard import semmle.code.java.dataflow.FlowSources private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking::Configuration { @@ -32,10 +31,6 @@ private class MyBatisAnnotationSqlInjectionConfiguration extends TaintTracking:: node.getType() instanceof NumberType } - override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { - guard instanceof ContainsSanitizer or guard instanceof EqualsSanitizer - } - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { exists(MethodAccess ma | ma.getMethod().getDeclaringType() instanceof MapType and @@ -60,5 +55,5 @@ where cfg.hasFlowPath(source, sink) and isMybatisAnnotationSqlInjection(sink.getNode(), isoa) select sink.getNode(), source, sink, - "MyBatis annotation sql injection might include code from $@ to $@.", source.getNode(), - "this user input", isoa, "this sql operation" + "MyBatis annotation SQL injection might include code from $@ to $@.", source.getNode(), + "this user input", isoa, "this SQL operation" diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjectionLib.qll index f3f6475703b..6c834daf205 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisAnnotationSqlInjectionLib.qll @@ -1,3 +1,7 @@ +/** + * Provides classes for SQL injection detection in MyBatis annotation. + */ + import java import MyBatisCommonLib import semmle.code.xml.MyBatisMapperXML @@ -7,7 +11,7 @@ import semmle.code.java.frameworks.Properties /** A sink for MyBatis annotation method call an argument. */ class MyBatisAnnotationMethodCallAnArgument extends DataFlow::Node { MyBatisAnnotationMethodCallAnArgument() { - exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma | ma.getMethod() = msoam | + exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma | ma.getMethod() = msoam | ma.getAnArgument() = this.asExpr() ) } @@ -15,7 +19,7 @@ class MyBatisAnnotationMethodCallAnArgument extends DataFlow::Node { /** Get the #{...} or ${...} parameters in the Mybatis annotation value. */ private string getAnMybatiAnnotationSetValue(IbatisSqlOperationAnnotation isoa) { - result = isoa.getSqlValue().trim().regexpFind("(#|\\$)(\\{([^\\}]*\\}))", _, _) + result = isoa.getSqlValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _) } predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperationAnnotation isoa) { @@ -27,15 +31,15 @@ predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperatio // @Select(select id,name from test where name like '%${value}%') // Test test(String name); // ``` - exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, string res | + exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, string res | msoam = ma.getMethod() | msoam.getAnAnnotation() = isoa and res = getAnMybatiAnnotationSetValue(isoa) and msoam.getNumberOfParameters() = 1 and - not ma.getMethod().getAParameter().hasAnnotation() and + not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and res.matches("%${%}") and - not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and + not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and ma.getAnArgument() = node.asExpr() ) or @@ -47,11 +51,11 @@ predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperatio // @Select(select id,name from test where name like '%${value}%') // Test test(Map map); // ``` - exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res | + exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res | msoam = ma.getMethod() | msoam.getAnAnnotation() = isoa and - not ma.getMethod().getParameter(i).hasAnnotation() and + not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and ( ma.getMethod().getParameterType(i) instanceof MapType or ma.getMethod().getParameterType(i) instanceof ListType or @@ -59,7 +63,7 @@ predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperatio ) and res = getAnMybatiAnnotationSetValue(isoa) and res.matches("%${%}") and - not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and + not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and ma.getArgument(i) = node.asExpr() ) or @@ -71,13 +75,13 @@ predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperatio // @Select(select id,name from test order by ${name,jdbcType=VARCHAR}) // void test(Test test); // ``` - exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Class c | + exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, RefType t | msoam = ma.getMethod() | msoam.getAnAnnotation() = isoa and - not ma.getMethod().getParameter(i).hasAnnotation() and - ma.getMethod().getParameterType(i).getName() = c.getName() and - getAnMybatiAnnotationSetValue(isoa).matches("%${" + c.getAField().getName() + "%}") and + not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and + ma.getMethod().getParameterType(i).getName() = t.getName() and + getAnMybatiAnnotationSetValue(isoa).matches("%${" + t.getAField().getName() + "%}") and ma.getArgument(i) = node.asExpr() ) or @@ -89,12 +93,10 @@ predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperatio // @Select(select id,name from test order by ${orderby,jdbcType=VARCHAR}) // void test(@Param("orderby") String name); // ``` - exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Annotation annotation | - msoam = ma.getMethod() + exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, Annotation annotation | + msoam = ma.getMethod() and ma.getMethod().getParameter(i).getAnAnnotation() = annotation | msoam.getAnAnnotation() = isoa and - ma.getMethod().getParameter(i).hasAnnotation() and - ma.getMethod().getParameter(i).getAnAnnotation() = annotation and annotation.getType() instanceof TypeParam and getAnMybatiAnnotationSetValue(isoa) .matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() + @@ -109,18 +111,18 @@ predicate isMybatisAnnotationSqlInjection(DataFlow::Node node, IbatisSqlOperatio // @Select(select id,name from test order by ${arg0,jdbcType=VARCHAR}) // void test(String name); // ``` - exists(MybatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res | + exists(MyBatisSqlOperationAnnotationMethod msoam, MethodAccess ma, int i, string res | msoam = ma.getMethod() | msoam.getAnAnnotation() = isoa and - not ma.getMethod().getParameter(i).hasAnnotation() and + not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and res = getAnMybatiAnnotationSetValue(isoa) and ( res.matches("%${param" + (i + 1) + "%}") or res.matches("%${arg" + i + "%}") ) and - not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and + not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and ma.getArgument(i) = node.asExpr() ) } diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll index e5559c61eae..b4b6eb4981a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisCommonLib.qll @@ -1,3 +1,7 @@ +/** + * Provides public classes for MyBatis SQL injection detection. + */ + import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.frameworks.MyBatis @@ -28,15 +32,10 @@ private class PropertiesFlowConfig extends DataFlow2::Configuration { string getAnMybatisConfigurationVariableKey() { exists(PropertiesFlowConfig conf, DataFlow::Node n | propertiesKey(n, result) and - conf.hasFlow(_, n) + conf.hasFlowTo(n) ) } -/** The interface `org.apache.ibatis.annotations.Param`. */ -class TypeParam extends Interface { - TypeParam() { this.hasQualifiedName("org.apache.ibatis.annotations", "Param") } -} - /** A reference type that extends a parameterization of `java.util.List`. */ class ListType extends RefType { ListType() { diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.qhelp b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.qhelp index 6ff4d97b72c..3a1741d6f82 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.qhelp @@ -9,8 +9,7 @@ <

-When writing MyBatis mapping statements, try to use the format "#{xxx}". If you have to use parameters -such as "${xxx}", you must manually filter to prevent SQL injection attacks. +When writing MyBatis mapping statements, try to use the syntax #{xxx}. If the syntax ${xxx} must be used, any parameters included in it should be sanitized to prevent SQL injection attacks.

@@ -18,7 +17,7 @@ such as "${xxx}", you must manually filter to prevent SQL injection attacks.

The following examples show the bad situation and the good situation respectively. In bad1 and bad2 and bad3 and bad4 and bad5, the program -${ xxx} are dynamic SQL statements, these five examples of SQL injection vulnerabilities. In good1, +${xxx} are dynamic SQL statements, these five examples of SQL injection vulnerabilities. In good1, the program uses the ${xxx} dynamic feature SQL statement, but there are subtle restrictions on the data, and there is no SQL injection vulnerability.

diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql index 1fedf40a395..41dee9c890a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjection.ql @@ -1,12 +1,12 @@ /** - * @name MyBatis Mapper xml sql injection + * @name SQL injection in MyBatis Mapper XML * @description Constructing a dynamic SQL statement with input that comes from an * untrusted source could allow an attacker to modify the statement's * meaning or to execute arbitrary SQL commands. * @kind path-problem * @problem.severity error * @precision high - * @id java/sql-injection + * @id java/mybatis-xml-sql-injection * @tags security * external/cwe/cwe-089 */ @@ -15,7 +15,6 @@ import java import DataFlow::PathGraph import MyBatisMapperXmlSqlInjectionLib import semmle.code.xml.MyBatisMapperXML -import semmle.code.java.security.SanitizerGuard import semmle.code.java.dataflow.FlowSources private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::Configuration { @@ -33,10 +32,6 @@ private class MyBatisMapperXmlSqlInjectionConfiguration extends TaintTracking::C node.getType() instanceof NumberType } - override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { - guard instanceof ContainsSanitizer or guard instanceof EqualsSanitizer - } - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { exists(MethodAccess ma | ma.getMethod().getDeclaringType() instanceof MapType and @@ -61,5 +56,5 @@ where cfg.hasFlowPath(source, sink) and isMapperXmlSqlInjection(sink.getNode(), xmle) select sink.getNode(), source, sink, - "MyBatis Mapper XML sql injection might include code from $@ to $@.", source.getNode(), - "this user input", xmle, "this sql operation" + "MyBatis Mapper XML SQL injection might include code from $@ to $@.", source.getNode(), + "this user input", xmle, "this SQL operation" diff --git a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjectionLib.qll index 8fcca4c54b2..e3e5bff1baf 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjectionLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-089/MyBatisMapperXmlSqlInjectionLib.qll @@ -1,3 +1,7 @@ +/** + * Provide classes for SQL injection detection in MyBatis Mapper XML. + */ + import java import MyBatisCommonLib import semmle.code.xml.MyBatisMapperXML @@ -17,116 +21,120 @@ class MyBatisMapperMethodCallAnArgument extends DataFlow::Node { /** Get the #{...} or ${...} parameters in the Mybatis mapper xml file */ private string getAnMybatiXmlSetValue(XMLElement xmle) { - result = xmle.getTextValue().trim().regexpFind("(#|\\$)(\\{([^\\}]*\\}))", _, _) + result = xmle.getTextValue().trim().regexpFind("(#|\\$)\\{[^\\}]*\\}", _, _) } predicate isMapperXmlSqlInjection(DataFlow::Node node, XMLElement xmle) { // MyBatis Mapper method Param Annotation sql injection vulnerabilities. // 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}` - exists( - MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess mc, int i, - Annotation annotation - | - mbmxe.getMapperMethod() = mc.getMethod() + exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, int i, Annotation annotation | + mbmxe.getMapperMethod() = ma.getMethod() | ( mbmxe.getAChild*() = xmle or - mbmxe.getInclude().getRefid() = mbms.getId() and - mbms.getAChild*() = xmle + exists(MyBatisMapperSql mbms | + mbmxe.getInclude().getRefid() = mbms.getId() and + mbms.getAChild*() = xmle + ) ) and - mc.getMethod().getParameter(i).hasAnnotation() and - mc.getMethod().getParameter(i).getAnAnnotation() = annotation and + ma.getMethod().getParameter(i).getAnAnnotation() = annotation and annotation.getType() instanceof TypeParam and getAnMybatiXmlSetValue(xmle) .matches("%${" + annotation.getValue("value").(CompileTimeConstantExpr).getStringValue() + "%}") and - mc.getArgument(i) = node.asExpr() + ma.getArgument(i) = node.asExpr() ) or // MyBatis Mapper method Class Field sql injection vulnerabilities. // e.g. MyBatis Mapper method: `void test(Test test);` and MyBatis Mapper XML file:`select id,name from test order by ${name,jdbcType=VARCHAR}` - exists(MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess mc, int i, Class c | - mbmxe.getMapperMethod() = mc.getMethod() + exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, int i, RefType t | + mbmxe.getMapperMethod() = ma.getMethod() | ( mbmxe.getAChild*() = xmle or - mbmxe.getInclude().getRefid() = mbms.getId() and - mbms.getAChild*() = xmle + exists(MyBatisMapperSql mbms | + mbmxe.getInclude().getRefid() = mbms.getId() and + mbms.getAChild*() = xmle + ) ) and - not mc.getMethod().getParameter(i).hasAnnotation() and - mc.getMethod().getParameterType(i).getName() = c.getName() and - getAnMybatiXmlSetValue(xmle).matches("%${" + c.getAField().getName() + "%}") and - mc.getArgument(i) = node.asExpr() + not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and + ma.getMethod().getParameterType(i).getName() = t.getName() and + getAnMybatiXmlSetValue(xmle).matches("%${" + t.getAField().getName() + "%}") and + ma.getArgument(i) = node.asExpr() ) or // The parameter type of MyBatis Mapper method is Map or List or Array, which may cause SQL injection vulnerability. // e.g. MyBatis Mapper method: `void test(Map params);` and MyBatis Mapper XML file:`select id,name from test where name like '%${name}%'` exists( - MyBatisMapperSqlOperation mbmxe, MyBatisMapperForeach mbmf, MyBatisMapperSql mbms, - MethodAccess mc, int i, string res + MyBatisMapperSqlOperation mbmxe, MyBatisMapperForeach mbmf, MethodAccess ma, int i, string res | - mbmxe.getMapperMethod() = mc.getMethod() + mbmxe.getMapperMethod() = ma.getMethod() | mbmf = xmle and ( mbmxe.getAChild*() = xmle or - mbmxe.getInclude().getRefid() = mbms.getId() and - mbms.getAChild*() = xmle + exists(MyBatisMapperSql mbms | + mbmxe.getInclude().getRefid() = mbms.getId() and + mbms.getAChild*() = xmle + ) ) and - not mc.getMethod().getParameter(i).hasAnnotation() and + not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and ( - mc.getMethod().getParameterType(i) instanceof MapType or - mc.getMethod().getParameterType(i) instanceof ListType or - mc.getMethod().getParameterType(i) instanceof Array + ma.getMethod().getParameterType(i) instanceof MapType or + ma.getMethod().getParameterType(i) instanceof ListType or + ma.getMethod().getParameterType(i) instanceof Array ) and res = getAnMybatiXmlSetValue(xmle) and res.matches("%${%}") and - not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and - mc.getArgument(i) = node.asExpr() + not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and + ma.getArgument(i) = node.asExpr() ) or // SQL injection vulnerability where the MyBatis Mapper method has only one parameter and the parameter is not annotated with `@Param`. // e.g. MyBatis Mapper method: `void test(String name);` and MyBatis Mapper XML file:`select id,name from test where name like '%${value}%'` - exists(MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess mc, string res | - mbmxe.getMapperMethod() = mc.getMethod() + exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, string res | + mbmxe.getMapperMethod() = ma.getMethod() | ( mbmxe.getAChild*() = xmle or - mbmxe.getInclude().getRefid() = mbms.getId() and - mbms.getAChild*() = xmle + exists(MyBatisMapperSql mbms | + mbmxe.getInclude().getRefid() = mbms.getId() and + mbms.getAChild*() = xmle + ) ) and res = getAnMybatiXmlSetValue(xmle) and - mc.getMethod().getNumberOfParameters() = 1 and - not mc.getMethod().getAParameter().hasAnnotation() and + ma.getMethod().getNumberOfParameters() = 1 and + not ma.getMethod().getAParameter().getAnAnnotation().getType() instanceof TypeParam and res.matches("%${%}") and - not res.matches("${" + getAnMybatisConfigurationVariableKey() + "}") and - mc.getAnArgument() = node.asExpr() + not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and + ma.getAnArgument() = node.asExpr() ) or // MyBatis Mapper method default parameter sql injection vulnerabilities.the default parameter form of the method is arg[0...n] or param[1...n]. // e.g. MyBatis Mapper method: `void test(String name);` and MyBatis Mapper XML file:`select id,name from test where name like '%${arg0 or param1}%'` - exists( - MyBatisMapperSqlOperation mbmxe, MyBatisMapperSql mbms, MethodAccess mc, int i, string res - | - mbmxe.getMapperMethod() = mc.getMethod() + exists(MyBatisMapperSqlOperation mbmxe, MethodAccess ma, int i, string res | + mbmxe.getMapperMethod() = ma.getMethod() | ( mbmxe.getAChild*() = xmle or - mbmxe.getInclude().getRefid() = mbms.getId() and - mbms.getAChild*() = xmle + exists(MyBatisMapperSql mbms | + mbmxe.getInclude().getRefid() = mbms.getId() and + mbms.getAChild*() = xmle + ) ) and - not mc.getMethod().getParameter(i).hasAnnotation() and + not ma.getMethod().getParameter(i).getAnAnnotation().getType() instanceof TypeParam and res = getAnMybatiXmlSetValue(xmle) and ( res.matches("%${param" + (i + 1) + "%}") or res.matches("%${arg" + i + "%}") ) and - mc.getArgument(i) = node.asExpr() + not res = "${" + getAnMybatisConfigurationVariableKey() + "}" and + ma.getArgument(i) = node.asExpr() ) } diff --git a/java/ql/src/semmle/code/java/security/SanitizerGuard.qll b/java/ql/src/semmle/code/java/security/SanitizerGuard.qll deleted file mode 100644 index 618f80ba084..00000000000 --- a/java/ql/src/semmle/code/java/security/SanitizerGuard.qll +++ /dev/null @@ -1,33 +0,0 @@ -/** - * Provide universal sanitizer guards. - */ - -import java -import semmle.code.java.dataflow.DataFlow - -/** - * An contains method sanitizer guard. - * - * e.g. `if(test.contains("test")) {...` - */ -class ContainsSanitizer extends DataFlow::BarrierGuard { - ContainsSanitizer() { this.(MethodAccess).getMethod().hasName("contains") } - - override predicate checks(Expr e, boolean branch) { - e = this.(MethodAccess).getArgument(0) and branch = true - } -} - -/** - * An equals method sanitizer guard. - * - * e.g. `if("test".equals(test)) {...` - */ -class EqualsSanitizer extends DataFlow::BarrierGuard { - EqualsSanitizer() { this.(MethodAccess).getMethod().hasName("equals") } - - override predicate checks(Expr e, boolean branch) { - e = [this.(MethodAccess).getArgument(0), this.(MethodAccess).getQualifier()] and - branch = true - } -} diff --git a/java/ql/src/semmle/code/xml/MyBatisMapperXML.qll b/java/ql/src/semmle/code/xml/MyBatisMapperXML.qll index 0aa136246b0..e96ddd98277 100644 --- a/java/ql/src/semmle/code/xml/MyBatisMapperXML.qll +++ b/java/ql/src/semmle/code/xml/MyBatisMapperXML.qll @@ -1,3 +1,7 @@ +/** + * Provides classes for working with MyBatis mapper xml files and their content. + */ + import java /** @@ -20,12 +24,14 @@ class MyBatisMapperXMLElement extends XMLElement { /** * Gets the value for this element, with leading and trailing whitespace trimmed. */ - string getValue() { result = allCharactersString().trim() } + string getValue() { result = this.allCharactersString().trim() } /** * Gets the reference type bound to MyBatis Mapper XML File. */ - RefType getNamespaceRefType() { result.getQualifiedName() = getAttribute("namespace").getValue() } + RefType getNamespaceRefType() { + result.getQualifiedName() = this.getAttribute("namespace").getValue() + } } /** @@ -37,8 +43,11 @@ abstract class MyBatisMapperSqlOperation extends MyBatisMapperXMLElement { /** * Gets the `` element in a `MyBatisMapperSqlOperation`. */ - MyBatisMapperInclude getInclude() { result = getAChild*() } + MyBatisMapperInclude getInclude() { result = this.getAChild*() } + /** + * Gets the method bound to MyBatis Mapper XML File. + */ Method getMapperMethod() { result.getName() = this.getId() and result.getDeclaringType() = this.getParent().(MyBatisMapperXMLElement).getNamespaceRefType() @@ -49,72 +58,72 @@ abstract class MyBatisMapperSqlOperation extends MyBatisMapperXMLElement { * A `` element in a `MyBatisMapperSqlOperation`. */ class MyBatisMapperInsert extends MyBatisMapperSqlOperation { - MyBatisMapperInsert() { getName() = "insert" } + MyBatisMapperInsert() { this.getName() = "insert" } /** * Gets the value of the `id` attribute of this ``. */ - override string getId() { result = getAttribute("id").getValue() } + override string getId() { result = this.getAttribute("id").getValue() } } /** * A `` element in a `MyBatisMapperSqlOperation`. */ class MyBatisMapperUpdate extends MyBatisMapperSqlOperation { - MyBatisMapperUpdate() { getName() = "update" } + MyBatisMapperUpdate() { this.getName() = "update" } /** * Gets the value of the `id` attribute of this ``. */ - override string getId() { result = getAttribute("id").getValue() } + override string getId() { result = this.getAttribute("id").getValue() } } /** * A `` element in a `MyBatisMapperSqlOperation`. */ class MyBatisMapperDelete extends MyBatisMapperSqlOperation { - MyBatisMapperDelete() { getName() = "delete" } + MyBatisMapperDelete() { this.getName() = "delete" } /** * Gets the value of the `id` attribute of this ``. */ - override string getId() { result = getAttribute("id").getValue() } + override string getId() { result = this.getAttribute("id").getValue() } } /** * A ``. */ - override string getId() { result = getAttribute("id").getValue() } + override string getId() { result = this.getAttribute("id").getValue() } } /** * A ` select id,name from test where id = ${id} - - \ No newline at end of file