diff --git a/java/ql/test/query-tests/security/CWE-352/CsrfUnprotectedRequestTypeTest.java b/java/ql/test/query-tests/security/CWE-352/CsrfUnprotectedRequestTypeTest.java index cd0bfe26a641..9c93dd17d03f 100644 --- a/java/ql/test/query-tests/security/CWE-352/CsrfUnprotectedRequestTypeTest.java +++ b/java/ql/test/query-tests/security/CWE-352/CsrfUnprotectedRequestTypeTest.java @@ -22,6 +22,10 @@ import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.HttpRedirect; import org.kohsuke.stapler.HttpResponses; +import org.apache.ibatis.jdbc.SqlRunner; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import java.util.Map; @Controller public class CsrfUnprotectedRequestTypeTest { @@ -142,29 +146,46 @@ public void bad6() { // $ hasCsrfUnprotectedRequestType } catch (SQLException e) { } } + // BAD: allows request type not default-protected from CSRF when + // updating a database using `Statement.executeUpdate` @RequestMapping("/") public void badStatementExecuteUpdate() { // $ hasCsrfUnprotectedRequestType try { String item = "item"; String price = "price"; Statement statement = connection.createStatement(); - String query = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; - int count = statement.executeUpdate(query); + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + int count = statement.executeUpdate(sql); } catch (SQLException e) { } } + // BAD: allows request type not default-protected from CSRF when + // updating a database using `Statement.executeLargeUpdate` + @RequestMapping("/") + public void badStatementExecuteLargeUpdate() { // $ hasCsrfUnprotectedRequestType + try { + String item = "item"; + String price = "price"; + Statement statement = connection.createStatement(); + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + long count = statement.executeLargeUpdate(sql); + } catch (SQLException e) { } + } + + // BAD: allows request type not default-protected from CSRF when + // updating a database using `Statement.execute` with SQL UPDATE @RequestMapping("/") public void badStatementExecute() { // $ hasCsrfUnprotectedRequestType try { String item = "item"; String price = "price"; Statement statement = connection.createStatement(); - String query = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; - boolean bool = statement.execute(query); + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + boolean bool = statement.execute(sql); } catch (SQLException e) { } } - // GOOD: select not insert/update/delete + // GOOD: does not update a database, queries with SELECT @RequestMapping("/") public void goodStatementExecute() { try { @@ -176,6 +197,92 @@ public void goodStatementExecute() { } catch (SQLException e) { } } + // BAD: allows request type not default-protected from CSRF when + // updating a database using `SqlRunner.insert` + @RequestMapping("/") + public void badSqlRunnerInsert() { // $ hasCsrfUnprotectedRequestType + try { + String item = "item"; + String price = "price"; + String sql = "INSERT PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + SqlRunner sqlRunner = new SqlRunner(connection); + sqlRunner.insert(sql); + } catch (SQLException e) { } + } + + // BAD: allows request type not default-protected from CSRF when + // updating a database using `SqlRunner.update` + @RequestMapping("/") + public void badSqlRunnerUpdate() { // $ hasCsrfUnprotectedRequestType + try { + String item = "item"; + String price = "price"; + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + SqlRunner sqlRunner = new SqlRunner(connection); + sqlRunner.update(sql); + } catch (SQLException e) { } + } + + // BAD: allows request type not default-protected from CSRF when + // updating a database using `SqlRunner.delete` + @RequestMapping("/") + public void badSqlRunnerDelete() { // $ hasCsrfUnprotectedRequestType + try { + String item = "item"; + String price = "price"; + String sql = "DELETE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + SqlRunner sqlRunner = new SqlRunner(connection); + sqlRunner.delete(sql); + } catch (SQLException e) { } + } + + // BAD: allows request type not default-protected from CSRF when + // updating a database using `NamedParameterJdbcTemplate.update` + @RequestMapping("/") + public void badNamedParameterJdbcTemplateUpdate() { // $ hasCsrfUnprotectedRequestType + String item = "item"; + String price = "price"; + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + JdbcTemplate jdbcTemplate = new JdbcTemplate(); + NamedParameterJdbcTemplate nameParamjdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate); + nameParamjdbcTemplate.update(sql, null, null); + } + + // BAD: allows request type not default-protected from CSRF when + // updating a database using `NamedParameterJdbcTemplate.batchUpdate` + @RequestMapping("/") + public void badNamedParameterJdbcTemplateBatchUpdate() { // $ hasCsrfUnprotectedRequestType + String item = "item"; + String price = "price"; + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + JdbcTemplate jdbcTemplate = new JdbcTemplate(); + NamedParameterJdbcTemplate nameParamjdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate); + nameParamjdbcTemplate.batchUpdate(sql, (Map[]) null); + } + + // BAD: allows request type not default-protected from CSRF when + // updating a database using `NamedParameterJdbcTemplate.execute` + @RequestMapping("/") + public void badNamedParameterJdbcTemplateExecute() { // $ hasCsrfUnprotectedRequestType + String item = "item"; + String price = "price"; + String sql = "UPDATE PRODUCT SET PRICE='" + price + "' WHERE ITEM='" + item + "'"; + JdbcTemplate jdbcTemplate = new JdbcTemplate(); + NamedParameterJdbcTemplate nameParamjdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate); + nameParamjdbcTemplate.execute(sql, null); + } + + // GOOD: does not update a database, queries with SELECT + @RequestMapping("/") + public void goodNamedParameterJdbcTemplateExecute() { + String category = "category"; + String query = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" + + category + "' ORDER BY PRICE"; + JdbcTemplate jdbcTemplate = new JdbcTemplate(); + NamedParameterJdbcTemplate nameParamjdbcTemplate = new NamedParameterJdbcTemplate(jdbcTemplate); + nameParamjdbcTemplate.execute(query, null); + } + @Autowired private MyBatisService myBatisService; diff --git a/java/ql/test/query-tests/security/CWE-352/options b/java/ql/test/query-tests/security/CWE-352/options index 4a81432ec3e4..1fef01772fe9 100644 --- a/java/ql/test/query-tests/security/CWE-352/options +++ b/java/ql/test/query-tests/security/CWE-352/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.3.8/:${testdir}/../../../stubs/org.mybatis-3.5.4/:${testdir}/../../../stubs/stapler-1.263/:${testdir}/../../../stubs/javax-servlet-2.5:${testdir}/../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../stubs/saxon-xqj-9.x:${testdir}/../../../stubs/apache-commons-beanutils:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/apache-commons-lang:${testdir}/../../../stubs/jaxen-1.2.0 +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.3.8/:${testdir}/../../../stubs/org.mybatis-3.5.4/:${testdir}/../../../stubs/stapler-1.263/:${testdir}/../../../stubs/javax-servlet-2.5:${testdir}/../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../stubs/saxon-xqj-9.x:${testdir}/../../../stubs/apache-commons-beanutils:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/apache-commons-lang:${testdir}/../../../stubs/jaxen-1.2.0:${testdir}/../../../stubs/apache-commons-logging-1.2/ diff --git a/java/ql/test/stubs/org.mybatis-3.5.4/org/apache/ibatis/jdbc/SqlRunner.java b/java/ql/test/stubs/org.mybatis-3.5.4/org/apache/ibatis/jdbc/SqlRunner.java new file mode 100644 index 000000000000..7738de81e0e9 --- /dev/null +++ b/java/ql/test/stubs/org.mybatis-3.5.4/org/apache/ibatis/jdbc/SqlRunner.java @@ -0,0 +1,37 @@ +package org.apache.ibatis.jdbc; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +public class SqlRunner { + + public static final int NO_GENERATED_KEY = Integer.MIN_VALUE + 1001; + + private final Connection connection; + private boolean useGeneratedKeySupport; + + public SqlRunner(Connection connection) { + this.connection = connection; + } + + public void setUseGeneratedKeySupport(boolean useGeneratedKeySupport) { } + public Map selectOne(String sql, Object... args) throws SQLException { return null; } + public List> selectAll(String sql, Object... args) throws SQLException { return null; } + public int insert(String sql, Object... args) throws SQLException { return 0; } + public int update(String sql, Object... args) throws SQLException { return 0; } + public int delete(String sql, Object... args) throws SQLException { return 0; } + public void closeConnection() { } + private void setParameters(PreparedStatement ps, Object... args) throws SQLException { } + private List> getResults(ResultSet rs) throws SQLException { return null; } + +} diff --git a/java/ql/test/stubs/springframework-5.3.8/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java b/java/ql/test/stubs/springframework-5.3.8/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java new file mode 100644 index 000000000000..b5c37a6f86a2 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.3.8/org/springframework/jdbc/core/namedparam/NamedParameterJdbcTemplate.java @@ -0,0 +1,169 @@ +package org.springframework.jdbc.core.namedparam; + +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; +import java.util.stream.Stream; + +import javax.sql.DataSource; + +import org.springframework.dao.DataAccessException; +import org.springframework.jdbc.core.BatchPreparedStatementSetter; +import org.springframework.jdbc.core.JdbcOperations; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.PreparedStatementCallback; +import org.springframework.jdbc.core.PreparedStatementCreator; +import org.springframework.jdbc.core.ResultSetExtractor; +import org.springframework.jdbc.core.RowCallbackHandler; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.SqlParameter; +import org.springframework.jdbc.support.KeyHolder; +import org.springframework.jdbc.support.rowset.SqlRowSet; + +public class NamedParameterJdbcTemplate implements NamedParameterJdbcOperations { + + public static final int DEFAULT_CACHE_LIMIT = 256; + private final JdbcOperations classicJdbcTemplate; + public NamedParameterJdbcTemplate(DataSource dataSource) { + this.classicJdbcTemplate = new JdbcTemplate(dataSource); + } + public NamedParameterJdbcTemplate(JdbcOperations classicJdbcTemplate) { + this.classicJdbcTemplate = classicJdbcTemplate; + } + @Override + public JdbcOperations getJdbcOperations() { return null; } + public JdbcTemplate getJdbcTemplate() { return null; } + public void setCacheLimit(int cacheLimit) { } + public int getCacheLimit() { return 0; } + + @Override + public T execute(String sql, SqlParameterSource paramSource, PreparedStatementCallback action) + throws DataAccessException { return null; } + + @Override + public T execute(String sql, Map paramMap, PreparedStatementCallback action) + throws DataAccessException { return null; } + + @Override + public T execute(String sql, PreparedStatementCallback action) throws DataAccessException { return null; } + + @Override + public T query(String sql, SqlParameterSource paramSource, ResultSetExtractor rse) + throws DataAccessException { return null; } + + @Override + public T query(String sql, Map paramMap, ResultSetExtractor rse) + throws DataAccessException { return null; } + + @Override + public T query(String sql, ResultSetExtractor rse) throws DataAccessException { return null; } + + @Override + public void query(String sql, SqlParameterSource paramSource, RowCallbackHandler rch) + throws DataAccessException { } + + @Override + public void query(String sql, Map paramMap, RowCallbackHandler rch) + throws DataAccessException { } + + @Override + public void query(String sql, RowCallbackHandler rch) throws DataAccessException { } + + @Override + public List query(String sql, SqlParameterSource paramSource, RowMapper rowMapper) + throws DataAccessException { return null; } + + @Override + public List query(String sql, Map paramMap, RowMapper rowMapper) + throws DataAccessException { return null; } + + @Override + public List query(String sql, RowMapper rowMapper) throws DataAccessException { return null; } + + @Override + public Stream queryForStream(String sql, SqlParameterSource paramSource, RowMapper rowMapper) + throws DataAccessException { return null; } + + @Override + public Stream queryForStream(String sql, Map paramMap, RowMapper rowMapper) + throws DataAccessException { return null; } + + @Override + public T queryForObject(String sql, SqlParameterSource paramSource, RowMapper rowMapper) + throws DataAccessException { return null; } + + @Override + public T queryForObject(String sql, Map paramMap, RowMapperrowMapper) + throws DataAccessException { return null; } + + @Override + public T queryForObject(String sql, SqlParameterSource paramSource, Class requiredType) + throws DataAccessException { return null; } + + @Override + public T queryForObject(String sql, Map paramMap, Class requiredType) + throws DataAccessException { return null; } + + @Override + public Map queryForMap(String sql, SqlParameterSource paramSource) throws DataAccessException { return null; } + + @Override + public Map queryForMap(String sql, Map paramMap) throws DataAccessException { return null; } + + @Override + public List queryForList(String sql, SqlParameterSource paramSource, Class elementType) + throws DataAccessException { return null; } + + @Override + public List queryForList(String sql, Map paramMap, Class elementType) + throws DataAccessException { return null; } + + @Override + public List> queryForList(String sql, SqlParameterSource paramSource) + throws DataAccessException { return null; } + + @Override + public List> queryForList(String sql, Map paramMap) + throws DataAccessException { return null; } + + @Override + public SqlRowSet queryForRowSet(String sql, SqlParameterSource paramSource) throws DataAccessException { return null; } + + @Override + public SqlRowSet queryForRowSet(String sql, Map paramMap) throws DataAccessException { return null; } + + @Override + public int update(String sql, SqlParameterSource paramSource) throws DataAccessException { return 0; } + + @Override + public int update(String sql, Map paramMap) throws DataAccessException { return 0; } + + @Override + public int update(String sql, SqlParameterSource paramSource, KeyHolder generatedKeyHolder) + throws DataAccessException { return 0; } + + @Override + public int update( + String sql, SqlParameterSource paramSource, KeyHolder generatedKeyHolder, String[] keyColumnNames) + throws DataAccessException { return 0; } + + @Override + public int[] batchUpdate(String sql, SqlParameterSource[] batchArgs) { return new int[0]; } + + @Override + public int[] batchUpdate(String sql, Map[] batchValues) { return new int[0]; } + + public int[] batchUpdate(String sql, SqlParameterSource[] batchArgs, KeyHolder generatedKeyHolder) { return new int[0]; } + + public int[] batchUpdate(String sql, SqlParameterSource[] batchArgs, KeyHolder generatedKeyHolder, + String[] keyColumnNames) { return new int[0]; } + + protected PreparedStatementCreator getPreparedStatementCreator(String sql, SqlParameterSource paramSource) { + return null; + } + + protected ParsedSql getParsedSql(String sql) { return null; } + +}