diff --git a/src/main/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerability.java b/src/main/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerability.java index 749258ba..27371748 100644 --- a/src/main/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerability.java +++ b/src/main/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerability.java @@ -1,5 +1,7 @@ package org.sasanlabs.service.vulnerability.sqlInjection; +import java.sql.ResultSet; +import java.sql.SQLException; import java.util.Map; import org.sasanlabs.internal.utility.LevelConstants; import org.sasanlabs.internal.utility.Variant; @@ -25,10 +27,10 @@ value = "UnionBasedSQLInjectionVulnerability") public class UnionBasedSQLInjectionVulnerability { - private JdbcTemplate applicationJdbcTemplate; + private final JdbcTemplate applicationJdbcTemplate; public UnionBasedSQLInjectionVulnerability( - @Qualifier("applicationJdbcTemplate") JdbcTemplate applicationJdbcTemplate) { + @Qualifier("applicationJdbcTemplate") final JdbcTemplate applicationJdbcTemplate) { this.applicationJdbcTemplate = applicationJdbcTemplate; } @@ -40,19 +42,11 @@ public UnionBasedSQLInjectionVulnerability( value = LevelConstants.LEVEL_1, htmlTemplate = "LEVEL_1/SQLInjection_Level1") public ResponseEntity getCarInformationLevel1( - @RequestParam Map queryParams) { - String id = queryParams.get("id"); + @RequestParam final Map queryParams) { + final String id = queryParams.get("id"); return applicationJdbcTemplate.query( "select * from cars where id=" + id, - (rs) -> { - CarInformation carInformation = new CarInformation(); - if (rs.next()) { - carInformation.setId(rs.getInt(1)); - carInformation.setName(rs.getString(2)); - carInformation.setImagePath(rs.getString(3)); - } - return new ResponseEntity(carInformation, HttpStatus.OK); - }); + this::resultSetToResponse); } @AttackVector( @@ -64,19 +58,11 @@ public ResponseEntity getCarInformationLevel1( value = LevelConstants.LEVEL_2, htmlTemplate = "LEVEL_1/SQLInjection_Level1") public ResponseEntity getCarInformationLevel2( - @RequestParam Map queryParams) { - String id = queryParams.get("id"); - CarInformation carInformation = new CarInformation(); + @RequestParam final Map queryParams) { + final String id = queryParams.get("id"); return applicationJdbcTemplate.query( "select * from cars where id='" + id + "'", - (rs) -> { - if (rs.next()) { - carInformation.setId(rs.getInt(1)); - carInformation.setName(rs.getString(2)); - carInformation.setImagePath(rs.getString(3)); - } - return new ResponseEntity(carInformation, HttpStatus.OK); - }); + this::resultSetToResponse); } @AttackVector( @@ -88,19 +74,11 @@ public ResponseEntity getCarInformationLevel2( variant = Variant.SECURE, htmlTemplate = "LEVEL_1/SQLInjection_Level1") public ResponseEntity getCarInformationLevel3( - @RequestParam Map queryParams) { - String id = queryParams.get("id").replaceAll("'", ""); + @RequestParam final Map queryParams) { + final String id = queryParams.get("id").replaceAll("'", ""); return applicationJdbcTemplate.query( "select * from cars where id='" + id + "'", - (rs) -> { - CarInformation carInformation = new CarInformation(); - if (rs.next()) { - carInformation.setId(rs.getInt(1)); - carInformation.setName(rs.getString(2)); - carInformation.setImagePath(rs.getString(3)); - } - return new ResponseEntity(carInformation, HttpStatus.OK); - }); + this::resultSetToResponse); } @VulnerableAppRequestMapping( @@ -108,22 +86,22 @@ public ResponseEntity getCarInformationLevel3( variant = Variant.SECURE, htmlTemplate = "LEVEL_1/SQLInjection_Level1") public ResponseEntity getCarInformationLevel4( - @RequestParam Map queryParams) { - String id = queryParams.get("id"); + @RequestParam final Map queryParams) { + final String id = queryParams.get("id"); return applicationJdbcTemplate.query( "select * from cars where id=?", - (prepareStatement) -> { - prepareStatement.setString(1, id); - }, - (rs) -> { - CarInformation carInformation = new CarInformation(); - if (rs.next()) { - carInformation.setId(rs.getInt(1)); - carInformation.setName(rs.getString(2)); - carInformation.setImagePath(rs.getString(3)); - } - return new ResponseEntity(carInformation, HttpStatus.OK); - }); + prepareStatement -> prepareStatement.setString(1, id), + this::resultSetToResponse); + } + + private ResponseEntity resultSetToResponse(final ResultSet rs) throws SQLException { + final CarInformation carInformation = new CarInformation(); + if (rs.next()) { + carInformation.setId(rs.getInt(1)); + carInformation.setName(rs.getString(2)); + carInformation.setImagePath(rs.getString(3)); + } + return new ResponseEntity<>(carInformation, HttpStatus.OK); } } diff --git a/src/test/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerabilityTest.java b/src/test/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerabilityTest.java index 621b6909..8bc984d3 100644 --- a/src/test/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerabilityTest.java +++ b/src/test/java/org/sasanlabs/service/vulnerability/sqlInjection/UnionBasedSQLInjectionVulnerabilityTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.PreparedStatementSetter; import org.springframework.jdbc.core.ResultSetExtractor; import java.io.IOException; @@ -14,7 +15,7 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; -public class UnionBasedSQLInjectionVulnerabilityTest { +class UnionBasedSQLInjectionVulnerabilityTest { private UnionBasedSQLInjectionVulnerability unionBasedSQLInjectionVulnerability; private JdbcTemplate template; @@ -31,9 +32,9 @@ void setUp() throws IOException { } @Test - public void getCarInformationLevel1_ExpectParamInjected() throws IOException { + void getCarInformationLevel1_ExpectParamInjected() throws IOException { // Act - Map params = new HashMap(); + final Map params = new HashMap<>(); params.put("id", "1 UNION SELECT * FROM cars;"); unionBasedSQLInjectionVulnerability.getCarInformationLevel1(params); @@ -42,7 +43,7 @@ public void getCarInformationLevel1_ExpectParamInjected() throws IOException { } @Test - public void getCarInformationLevel2_ExpectParamInjected() throws IOException { + void getCarInformationLevel2_ExpectParamInjected() throws IOException { // Act Map params = new HashMap(); params.put("id", "1' UNION SELECT * FROM cars; --"); @@ -52,6 +53,39 @@ public void getCarInformationLevel2_ExpectParamInjected() throws IOException { verify(template).query(eq("select * from cars where id='1' UNION SELECT * FROM cars; --'"), (ResultSetExtractor) any()); } + @Test + void getCarInformationLevel3_ExpectParamEscaped() throws IOException { + // Act + Map params = new HashMap(); + params.put("id", "1' UNION SELECT * FROM cars; --"); + unionBasedSQLInjectionVulnerability.getCarInformationLevel3(params); + + // Assert + verify(template).query(eq("select * from cars where id='1 UNION SELECT * FROM cars; --'"), (ResultSetExtractor) any()); + + } + + @Test + void getCarInformationLevel4_ExpectParamEscaped() throws IOException { + // Setup + template = Mockito.spy(new JdbcTemplate()); + PreparedStatementSetter setter = (ps) -> {}; + doReturn(null) + .when(template) + .query(anyString(), (PreparedStatementSetter) any(), (ResultSetExtractor) any()); + + unionBasedSQLInjectionVulnerability = Mockito.spy(new UnionBasedSQLInjectionVulnerability(template)); + + // Act + Map params = new HashMap(); + params.put("id", "1' UNION SELECT * FROM cars; --"); + unionBasedSQLInjectionVulnerability.getCarInformationLevel4(params); + + // Assert + verify(template).query(eq("select * from cars where id=?"), (PreparedStatementSetter) any(), (ResultSetExtractor) any()); + + } + // private JdbcTemplate applicationJdbcTemplate; // // public UnionBasedSQLInjectionVulnerabilityTest(