diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java
index 3df08175a..1128e2fec 100644
--- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java
+++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java
@@ -63,36 +63,42 @@ public class SqlInjectionLesson9 extends AssignmentEndpoint {
protected AttackResult injectableQueryIntegrity(String name, String auth_tan) {
StringBuilder output = new StringBuilder();
- String query =
+ String queryInjection =
"SELECT * FROM employees WHERE last_name = '"
+ name
+ "' AND auth_tan = '"
+ auth_tan
+ "'";
try (Connection connection = dataSource.getConnection()) {
- try {
- Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE);
- SqlInjectionLesson8.log(connection, query);
- ResultSet results = statement.executeQuery(query);
- var test = results.getRow() != 0;
- if (results.getStatement() != null) {
- if (results.first()) {
- output.append(SqlInjectionLesson8.generateTable(results));
- } else {
- // no results
- return failed(this).feedback("sql-injection.8.no.results").build();
- }
- }
- } catch (SQLException e) {
- System.err.println(e.getMessage());
- return failed(this)
- .output("
" + e.getMessage() + "")
- .build();
+ // V2019_09_26_7__employees.sql
+ int oldMaxSalary = this.getMaxSalary(connection);
+ int oldSumSalariesOfOtherEmployees = this.getSumSalariesOfOtherEmployees(connection);
+ // begin transaction
+ connection.setAutoCommit(false);
+ // do injectable query
+ Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE);
+ SqlInjectionLesson8.log(connection, queryInjection);
+ statement.execute(queryInjection);
+ // check new sum of salaries other employees and new salaries of John
+ int newJohnSalary = this.getJohnSalary(connection);
+ int newSumSalariesOfOtherEmployees = this.getSumSalariesOfOtherEmployees(connection);
+ if (newJohnSalary > oldMaxSalary
+ && newSumSalariesOfOtherEmployees == oldSumSalariesOfOtherEmployees) {
+ // success commit
+ connection.commit(); // need execute not executeQuery
+ connection.setAutoCommit(true);
+ output.append(
+ SqlInjectionLesson8.generateTable(this.getEmployeesDataOrderBySalaryDesc(connection)));
+ return success(this).feedback("sql-injection.9.success").output(output.toString()).build();
}
-
- return checkSalaryRanking(connection, output);
-
- } catch (Exception e) {
+ // failed roolback
+ connection.rollback();
+ return failed(this)
+ .feedback("sql-injection.9.one")
+ .output(
+ SqlInjectionLesson8.generateTable(this.getEmployeesDataOrderBySalaryDesc(connection)))
+ .build();
+ } catch (SQLException e) {
System.err.println(e.getMessage());
return failed(this)
.output("
" + e.getMessage() + "")
@@ -100,29 +106,31 @@ public class SqlInjectionLesson9 extends AssignmentEndpoint {
}
}
- private AttackResult checkSalaryRanking(Connection connection, StringBuilder output) {
- try {
- String query = "SELECT * FROM employees ORDER BY salary DESC";
- try (Statement statement =
- connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); ) {
- ResultSet results = statement.executeQuery(query);
+ private int getSqlInt(Connection connection, String query) throws SQLException {
+ Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE);
+ ResultSet results = statement.executeQuery(query);
+ results.first();
+ return results.getInt(1);
+ }
- results.first();
- // user completes lesson if John Smith is the first in the list
- if ((results.getString(2).equals("John")) && (results.getString(3).equals("Smith"))) {
- output.append(SqlInjectionLesson8.generateTable(results));
- return success(this)
- .feedback("sql-injection.9.success")
- .output(output.toString())
- .build();
- } else {
- return failed(this).feedback("sql-injection.9.one").output(output.toString()).build();
- }
- }
- } catch (SQLException e) {
- return failed(this)
- .output("
" + e.getMessage() + "")
- .build();
- }
+ private int getMaxSalary(Connection connection) throws SQLException {
+ String query = "SELECT max(salary) FROM employees";
+ return this.getSqlInt(connection, query);
+ }
+
+ private int getSumSalariesOfOtherEmployees(Connection connection) throws SQLException {
+ String query = "SELECT sum(salary) FROM employees WHERE auth_tan != '3SL99A'";
+ return this.getSqlInt(connection, query);
+ }
+
+ private int getJohnSalary(Connection connection) throws SQLException {
+ String query = "SELECT salary FROM employees WHERE auth_tan = '3SL99A'";
+ return this.getSqlInt(connection, query);
+ }
+
+ private ResultSet getEmployeesDataOrderBySalaryDesc(Connection connection) throws SQLException {
+ String query = "SELECT * FROM employees ORDER BY salary DESC";
+ Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE);
+ return statement.executeQuery(query);
}
}
diff --git a/src/main/resources/lessons/sqlinjection/i18n/WebGoatLabels.properties b/src/main/resources/lessons/sqlinjection/i18n/WebGoatLabels.properties
index b702537ca..aca652d13 100644
--- a/src/main/resources/lessons/sqlinjection/i18n/WebGoatLabels.properties
+++ b/src/main/resources/lessons/sqlinjection/i18n/WebGoatLabels.properties
@@ -63,8 +63,8 @@ SqlStringInjectionHint.8.3=Try appending a SQL statement that always resolves to
SqlStringInjectionHint.8.4=Make sure all quotes (" ' ") are opened and closed properly so the resulting SQL query is syntactically correct.
SqlStringInjectionHint.8.5=Try extending the WHERE clause of the statement by adding something like: ' OR '1' = '1.
-sql-injection.9.success=Well done! Now you are earning the most money. And at the same time you successfully compromised the integrity of data by changing the salary!
-sql-injection.9.one=Still not earning enough! Better try again and change that.
+sql-injection.9.success=Well done! Now you are earning the most money. And at the same time you successfully compromised the integrity of data by changing your salary!
+sql-injection.9.one=Still not earning enough or only your own salary must be changed! Better try again and change that.
SqlStringInjectionHint.9.1=Try to find a way, to chain another query to the end of the existing one.
SqlStringInjectionHint.9.2=Use the ; metacharacter to do so.
SqlStringInjectionHint.9.3=Make use of DML to change your salary.
diff --git a/src/test/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9Test.java b/src/test/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9Test.java
index 4cd0c368f..44438f6c0 100644
--- a/src/test/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9Test.java
+++ b/src/test/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9Test.java
@@ -37,123 +37,7 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
*/
public class SqlInjectionLesson9Test extends SqlLessonTest {
- private String completedError = "JSON path \"lessonCompleted\"";
-
- @Test
- public void oneAccount() throws Exception {
- try {
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smith")
- .param("auth_tan", "3SL99A"))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(false)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.one"))))
- .andExpect(jsonPath("$.output", containsString("
")));
- } catch (AssertionError e) {
- if (!e.getMessage().contains(completedError)) throw e;
-
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smith")
- .param("auth_tan", "3SL99A"))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(true)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.success"))))
- .andExpect(jsonPath("$.output", containsString("")));
- }
- }
-
- @Test
- public void multipleAccounts() throws Exception {
- try {
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smith")
- .param("auth_tan", "3SL99A' OR '1' = '1"))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(false)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.one"))))
- .andExpect(
- jsonPath(
- "$.output",
- containsString(
- " |
---|
96134<\\/td> | Bob<\\/td> | Franco<\\/td> | Marketing<\\/td> | 83700<\\/td> | LO9S2V<\\/td><\\/tr>")));
- } catch (AssertionError e) {
- if (!e.getMessage().contains(completedError)) throw e;
-
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smith")
- .param("auth_tan", "3SL99A' OR '1' = '1"))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(true)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.success"))))
- .andExpect(
- jsonPath(
- "$.output",
- containsString(
- " | 96134<\\/td> | Bob<\\/td> | Franco<\\/td> | Marketing<\\/td> | 83700<\\/td> | LO9S2V<\\/td><\\/tr>")));
- }
- }
-
- @Test
- public void wrongNameReturnsNoAccounts() throws Exception {
- try {
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smithh")
- .param("auth_tan", "3SL99A"))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(false)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.8.no.results"))))
- .andExpect(jsonPath("$.output").doesNotExist());
- } catch (AssertionError e) {
- if (!e.getMessage().contains(completedError)) throw e;
-
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smithh")
- .param("auth_tan", "3SL99A"))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(true)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.8.no.success"))))
- .andExpect(jsonPath("$.output").doesNotExist());
- }
- }
-
- @Test
- public void wrongTANReturnsNoAccounts() throws Exception {
- try {
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smithh")
- .param("auth_tan", ""))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(false)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.8.no.results"))))
- .andExpect(jsonPath("$.output").doesNotExist());
- } catch (AssertionError e) {
- if (!e.getMessage().contains(completedError)) throw e;
-
- mockMvc
- .perform(
- MockMvcRequestBuilders.post("/SqlInjection/attack9")
- .param("name", "Smithh")
- .param("auth_tan", ""))
- .andExpect(status().isOk())
- .andExpect(jsonPath("lessonCompleted", is(true)))
- .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.success"))))
- .andExpect(jsonPath("$.output").doesNotExist());
- }
- }
+ private final String completedError = "JSON path \"lessonCompleted\"";
@Test
public void malformedQueryReturnsError() throws Exception {
@@ -181,6 +65,44 @@ public class SqlInjectionLesson9Test extends SqlLessonTest {
}
}
+ @Test
+ public void SmithIsNotMostEarning() throws Exception {
+ mockMvc
+ .perform(
+ MockMvcRequestBuilders.post("/SqlInjection/attack9")
+ .param("name", "Smith")
+ .param(
+ "auth_tan",
+ "3SL99A'; UPDATE employees SET salary = 9999 WHERE last_name = 'Smith"))
+ .andExpect(status().isOk())
+ .andExpect(jsonPath("lessonCompleted", is(false)))
+ .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.one"))));
+ }
+
+ @Test
+ public void OnlySmithSalaryMustBeUpdated() throws Exception {
+ mockMvc
+ .perform(
+ MockMvcRequestBuilders.post("/SqlInjection/attack9")
+ .param("name", "Smith")
+ .param("auth_tan", "3SL99A'; UPDATE employees SET salary = 9999 -- "))
+ .andExpect(status().isOk())
+ .andExpect(jsonPath("lessonCompleted", is(false)))
+ .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.one"))));
+ }
+
+ @Test
+ public void OnlySmithMustMostEarning() throws Exception {
+ mockMvc
+ .perform(
+ MockMvcRequestBuilders.post("/SqlInjection/attack9")
+ .param("name", "'; UPDATE employees SET salary = 999999 -- ")
+ .param("auth_tan", ""))
+ .andExpect(status().isOk())
+ .andExpect(jsonPath("lessonCompleted", is(false)))
+ .andExpect(jsonPath("$.feedback", is(messages.getMessage("sql-injection.9.one"))));
+ }
+
@Test
public void SmithIsMostEarningCompletesAssignment() throws Exception {
mockMvc
|
|
---|