From 256c0d05aa30aecd0404dc646fa8b8ad3f135972 Mon Sep 17 00:00:00 2001 From: Tobias-Melzer Date: Mon, 4 Feb 2019 22:45:56 +0100 Subject: [PATCH] Implemented some feedback --- .../resources/i18n/WebGoatLabels.properties | 5 +++ .../plugin/advanced/SqlInjectionLesson6a.java | 33 +++++++++---------- .../introduction/SqlInjectionLesson5a.java | 29 ++++++++-------- .../src/main/resources/html/SqlInjection.html | 4 +-- .../resources/i18n/WebGoatLabels.properties | 5 +-- .../lessonPlans/en/SqlInjection_content6.adoc | 30 ++++++++++++++--- .../en/SqlInjection_content6a.adoc | 7 ++-- .../SqlInjection_introduction_content11.adoc | 2 +- .../SqlInjection_introduction_content12.adoc | 2 +- 9 files changed, 73 insertions(+), 44 deletions(-) diff --git a/webgoat-lessons/chrome-dev-tools/src/main/resources/i18n/WebGoatLabels.properties b/webgoat-lessons/chrome-dev-tools/src/main/resources/i18n/WebGoatLabels.properties index 3741c9cf5..0754626c1 100644 --- a/webgoat-lessons/chrome-dev-tools/src/main/resources/i18n/WebGoatLabels.properties +++ b/webgoat-lessons/chrome-dev-tools/src/main/resources/i18n/WebGoatLabels.properties @@ -7,5 +7,10 @@ network.request=You made a HTTP Request. network.success=Correct, Well Done. network.failed=That is not correct, try again. +<<<<<<< HEAD networkHint1=Clear all Requests from the network button, then make the request. The you should be able to figure out, which request holds the data. networkHint2=The name of the request is "dummy" +======= +networkHint1=Clear all Requests from the network tab, then make the request. The you should be able to figure out, which request holds the data. +networkHint2=The name of the request is "dummy" +>>>>>>> Implemented some feedback diff --git a/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/advanced/SqlInjectionLesson6a.java b/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/advanced/SqlInjectionLesson6a.java index 157e21606..e72c74577 100644 --- a/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/advanced/SqlInjectionLesson6a.java +++ b/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/advanced/SqlInjectionLesson6a.java @@ -6,10 +6,7 @@ import org.owasp.webgoat.assignments.AssignmentPath; import org.owasp.webgoat.assignments.AttackResult; import org.owasp.webgoat.plugin.introduction.SqlInjectionLesson5a; import org.owasp.webgoat.session.DatabaseUtilities; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.*; import java.io.IOException; import java.sql.*; @@ -47,10 +44,10 @@ import java.sql.*; */ @AssignmentPath("/SqlInjection/attack6a") @AssignmentHints(value = {"SqlStringInjectionHint-advanced-6a-1", "SqlStringInjectionHint-advanced-6a-2", "SqlStringInjectionHint-advanced-6a-3", -"SqlStringInjectionHint-advanced-6a-4", "SqlStringInjectionHint-advanced-6a-5"}) +"SqlStringInjectionHint-advanced-6a-4"}) public class SqlInjectionLesson6a extends AssignmentEndpoint { - @RequestMapping(method = RequestMethod.POST) + @PostMapping public @ResponseBody AttackResult completed(@RequestParam String userid_6a) throws IOException { @@ -60,17 +57,15 @@ public class SqlInjectionLesson6a extends AssignmentEndpoint { protected AttackResult injectableQuery(String accountName) { String query = ""; - try { + try(Connection connection = DatabaseUtilities.getConnection(getWebSession())) { boolean usedUnion = true; - Connection connection = DatabaseUtilities.getConnection(getWebSession()); query = "SELECT * FROM user_data WHERE last_name = '" + accountName + "'"; //Check if Union is used if(!accountName.matches("(?i)(^[^-/*;)]*)(\\s*)UNION(.*$)")) { usedUnion = false; } - try { - Statement statement = connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, - ResultSet.CONCUR_READ_ONLY); + try(Statement statement = connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, + ResultSet.CONCUR_READ_ONLY)) { ResultSet results = statement.executeQuery(query); if ((results != null) && (results.first())) { @@ -78,15 +73,17 @@ public class SqlInjectionLesson6a extends AssignmentEndpoint { StringBuffer output = new StringBuffer(); output.append(SqlInjectionLesson5a.writeTable(results, resultsMetaData)); - if(! usedUnion) - output.append("To successfully complete this Assignment you have to use a UNION"); + + String appendingWhenSucceded; + if(usedUnion) + appendingWhenSucceded = "Well done! Can you also figure out a solution, by appending a new Sql Statement?"; + else + appendingWhenSucceded = "Well done! Can you also figure out a solution, by using a UNION?"; results.last(); - // If they get back more than one user they succeeded - if (results.getRow() >= 5 && usedUnion) { - return trackProgress(success().feedback("sql-injection.advanced.6a.success").feedbackArgs(output.toString()).output(" Your query was: " + query).build()); - } else if((output.toString().contains("dave") && output.toString().contains("passW0rD")) && !usedUnion) { - return trackProgress(failed().output("To successfully complete this Assignment you have to use a UNION" + "
Your query was: " + query).build()); + if (output.toString().contains("dave") && output.toString().contains("passW0rD")) { + output.append(appendingWhenSucceded); + return trackProgress(informationMessage().feedback("sql-injection.advanced.6a.success").feedbackArgs(output.toString()).output(" Your query was: " + query).build()); } else { return trackProgress(failed().output(output.toString() + "
Your query was: " + query).build()); } diff --git a/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/introduction/SqlInjectionLesson5a.java b/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/introduction/SqlInjectionLesson5a.java index a588ce806..58ea415d9 100644 --- a/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/introduction/SqlInjectionLesson5a.java +++ b/webgoat-lessons/sql-injection/src/main/java/org/owasp/webgoat/plugin/introduction/SqlInjectionLesson5a.java @@ -5,10 +5,7 @@ import org.owasp.webgoat.assignments.AssignmentHints; import org.owasp.webgoat.assignments.AssignmentPath; import org.owasp.webgoat.assignments.AttackResult; import org.owasp.webgoat.session.DatabaseUtilities; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.*; import java.io.IOException; import java.sql.*; @@ -45,9 +42,15 @@ import java.sql.*; * @created October 28, 2003 */ @AssignmentPath("/SqlInjection/assignment5a") +@AssignmentHints(value = {"SqlStringInjectionHint5a1"}) public class SqlInjectionLesson5a extends AssignmentEndpoint { - @RequestMapping(method = RequestMethod.POST) + private static final String EXPLANATION = "
Explanation: This injection works, because or '1' = '1' " + + "always evaluates to true (The string ending literal for '1 is closed by the query itself, so you should not inject it). " + + "So the injected query basically looks like this: SELECT * FROM user_data WHERE first_name = 'John' and last_name = '' or TRUE, " + + "which will always evaluate to true, no matter what came before it."; + + @PostMapping public @ResponseBody AttackResult completed(@RequestParam String account, @RequestParam String operator, @RequestParam String injection) { @@ -58,23 +61,22 @@ public class SqlInjectionLesson5a extends AssignmentEndpoint { String query = ""; try { Connection connection = DatabaseUtilities.getConnection(getWebSession()); - System.out.println(accountName); query = "SELECT * FROM user_data WHERE first_name = 'John' and last_name = '" + accountName + "'"; - try { - Statement statement = connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, - ResultSet.CONCUR_READ_ONLY); + try(Statement statement = connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, + ResultSet.CONCUR_READ_ONLY)) { + ResultSet results = statement.executeQuery(query); if ((results != null) && (results.first())) { ResultSetMetaData resultsMetaData = results.getMetaData(); - StringBuffer output = new StringBuffer(); + StringBuilder output = new StringBuilder(); output.append(writeTable(results, resultsMetaData)); results.last(); // If they get back more than one user they succeeded if (results.getRow() >= 6) { - return trackProgress(success().feedback("sql-injection.5a.success").output("Your query was: " + query).feedbackArgs(output.toString()).build()); + return trackProgress(success().feedback("sql-injection.5a.success").output("Your query was: " + query + EXPLANATION).feedbackArgs(output.toString()).build()); } else { return trackProgress(failed().output(output.toString() + "
Your query was: " + query).build()); } @@ -91,11 +93,10 @@ public class SqlInjectionLesson5a extends AssignmentEndpoint { } } - public static String writeTable(ResultSet results, ResultSetMetaData resultsMetaData) throws IOException, - SQLException { + public static String writeTable(ResultSet results, ResultSetMetaData resultsMetaData) throws SQLException { int numColumns = resultsMetaData.getColumnCount(); results.beforeFirst(); - StringBuffer t = new StringBuffer(); + StringBuilder t = new StringBuilder(); t.append("

"); if (results.next()) { diff --git a/webgoat-lessons/sql-injection/src/main/resources/html/SqlInjection.html b/webgoat-lessons/sql-injection/src/main/resources/html/SqlInjection.html index d10f11156..862a35991 100644 --- a/webgoat-lessons/sql-injection/src/main/resources/html/SqlInjection.html +++ b/webgoat-lessons/sql-injection/src/main/resources/html/SqlInjection.html @@ -151,7 +151,7 @@ enctype="application/json;charset=UTF-8"> - +
SELECT * FROM users WHERE LOGIN_COUNT > 0 and FIRST_NAME = 'SELECT * FROM users_data FIRST_NAME = 'John' and Last_NAME = ' diff --git a/webgoat-lessons/sql-injection/src/main/resources/i18n/WebGoatLabels.properties b/webgoat-lessons/sql-injection/src/main/resources/i18n/WebGoatLabels.properties index 5fe135699..b6e9a02b7 100644 --- a/webgoat-lessons/sql-injection/src/main/resources/i18n/WebGoatLabels.properties +++ b/webgoat-lessons/sql-injection/src/main/resources/i18n/WebGoatLabels.properties @@ -28,9 +28,10 @@ SqlStringInjectionHint5-a=Look at the example. There is everything you will need sql-injection.5a.success= sql-injection.5a.no.results= +SqlStringInjectionHint5a1=Remember that for an successful Sql-Injection the query needs to always evaluate to true. + sql-injection.5b.success= sql-injection.5b.no.results= - SqlStringInjectionHint5b1=Try to check which of the input fields is susceptible to an injection attack. SqlStringInjectionHint5b2=Insert: 0 or 1 = 1 into the first input field. The output should tell you if this field is injectable. SqlStringInjectionHint5b3=The first input field is not susceptible to sql injection. @@ -45,7 +46,7 @@ SqlStringInjectionHint-advanced-6a-1=Remember that when using an UNION each SELE SqlStringInjectionHint-advanced-6a-2=The data type of a column in the first SELECT statement must have a similar data type to that in the second SELECT statement. SqlStringInjectionHint-advanced-6a-3=Your new SQL query must end with a comment. eg: -- SqlStringInjectionHint-advanced-6a-4=If a column needs a String you could substitute something like 'a String' for it. For integers you could substitute a 1. -SqlStringInjectionHint-advanced-6a-5=Try something like: Smith' UNION SELECT userid,user_name, password, 'a', 'b', 'c', 1 from user_system_data -- +SqlStringInjectionHint-advanced-6a-5=Try something like: Smith' UNION SELECT userid,user_name, password, 'a', 'b', 'c', 1 from user_system_data -- sql-injection.6b.success= sql-injection.6b.no.results= diff --git a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6.adoc b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6.adoc index 0ff4ece16..50a864f35 100644 --- a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6.adoc +++ b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6.adoc @@ -27,8 +27,30 @@ Example: Select * from users where name = '+char(27) or 1=1 == Special Statements -Unions allows overlapping of database tables -'Select id, text from news -union all select name, pass from users' +=== Union -Joins allows connecting to other tables +The Union operator is used, to combine the results of two or more SELECT Statements. + +Rules to keep in mind, when working with a UNION: + +- The number of columns selected in each statement must be the same. +- The datatype of the first column in the first SELECT statement, must match the datatype +of the first column in the second (third, fourth, ...) SELECT Statement. The Same applies to all other columns. + +[source] +------ +SELECT First_Name from user_system_data UNION SELECT login_count FROM user_data; +------ + +The UNION ALL Syntax also allows duplicate Values. + +=== Joins + +The Join operator is used to combine rows from two ore more tables, based on a related column + +[source] +----- +SELECT * FROM user_data INNER JOIN user_data_tan ON user_data.userid=user_data_tan.userid; +----- + +For more detailed information about JOINS visit: https://www.w3schools.com/sql/sql_join.asp \ No newline at end of file diff --git a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6a.adoc b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6a.adoc index a959af2d8..2ce8dcced 100644 --- a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6a.adoc +++ b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_content6a.adoc @@ -24,5 +24,8 @@ CREATE TABLE user_system_data (userid int not null primary key, cookie varchar(30)); ------------------------------------------------------- -*6.a)* Retrieve all data from the table by using a UNION (You have to use a union to complete this assignment.) + -*6.b)* When you have figured it out.... What is Dave's password? \ No newline at end of file +*6.a)* Retrieve all data from the table + +*6.b)* When you have figured it out.... What is Dave's password? + +Note: There are multiple ways to solve this Assignment. One is by using a UNION, the other by appending +a new SQl statement. Maybe you can find both of them. \ No newline at end of file diff --git a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content11.adoc b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content11.adoc index b31eff4cc..a70446fc7 100644 --- a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content11.adoc +++ b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content11.adoc @@ -3,7 +3,7 @@ The query in the code builds a dynamic query as seen in the previous example. The query is build by concatenating strings making it susceptible to String SQL injection: ------------------------------------------------------------ -"select * from users where LOGIN_COUNT > 0 and FIRST_NAME = ‘" + userName + "'"; +"select * from user_data where FIRST_NAME = 'John' and LAST_NAME = '" + lastName + "'"; ------------------------------------------------------------ Using the form below try to retrieve all the users from the users table. You should not need to know any specific user name to get the complete list. diff --git a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content12.adoc b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content12.adoc index 946cda7e7..65f21db38 100644 --- a/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content12.adoc +++ b/webgoat-lessons/sql-injection/src/main/resources/lessonPlans/en/SqlInjection_introduction_content12.adoc @@ -3,7 +3,7 @@ The query in the code builds a dynamic query as seen in the previous example. The query in the code builds a dynamic query by concatenating a number making it susceptible to Numeric SQL injection: -------------------------------------------------- -"select * from users where Login_Count = " + Login_Count + " and USERID = " + UserID; +"select * from user_data where Login_Count = " + Login_Count + " and USERID = " + User_ID; -------------------------------------------------- Using the two Input Fields below, try to retrieve all the date from the users table.