From 137b7c813c74809d7cc0db4f6c9e17c17cb5ea90 Mon Sep 17 00:00:00 2001 From: "rogan.dawes" Date: Thu, 10 Jan 2008 10:11:50 +0000 Subject: [PATCH] several minor bug fixes. UpdateProfile uses prepared statements. ReflectedXSS "code" input field vulnerable to XSS. Minor updates to concurrency cart git-svn-id: http://webgoat.googlecode.com/svn/trunk@235 4033779f-a91e-0410-96ef-6bf7bf53c507 --- .../webgoat/lessons/ConcurrencyCart.java | 30 ++-- .../CrossSiteScripting/UpdateProfile.java | 146 ++++++++++++++---- .../owasp/webgoat/lessons/ReflectedXSS.java | 6 +- .../RoleBasedAccessControl/UpdateProfile.java | 110 ++++++++----- .../project/WebContent/webgoat_challenge.jsp | 12 +- 5 files changed, 207 insertions(+), 97 deletions(-) diff --git a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ConcurrencyCart.java b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ConcurrencyCart.java index 3f2a0d381..483617663 100644 --- a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ConcurrencyCart.java +++ b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ConcurrencyCart.java @@ -63,10 +63,10 @@ public class ConcurrencyCart extends LessonAdapter private static int runningTOTAL = 0; private static int subTOTAL = 0; private static int calcTOTAL = 0; - private static int quantity1 = 1; - private static int quantity2 = 1; - private static int quantity3 = 1; - private static int quantity4 = 1; + private static int quantity1 = 0; + private static int quantity2 = 0; + private static int quantity3 = 0; + private static int quantity4 = 0; private int discount = 0; public final static A ASPECT_LOGO = new A().setHref("http://www.aspectsecurity.com").addElement(new IMG("images/logos/aspect.jpg").setAlt("Aspect Security").setBorder(0).setHspace(0).setVspace(0)); @@ -108,7 +108,7 @@ public class ConcurrencyCart extends LessonAdapter else { //ALMOST - s.setMessage("Almost! You payed too much."); + //s.setMessage("Almost! You payed too much."); } } else @@ -131,10 +131,10 @@ public class ConcurrencyCart extends LessonAdapter //UPDATE QUANTITY VARIABLES private void updateQuantity(WebSession s) { - quantity1 = s.getParser().getIntParameter("QTY1", 1); - quantity2 = s.getParser().getIntParameter("QTY2", 1); - quantity3 = s.getParser().getIntParameter("QTY3", 1); - quantity4 = s.getParser().getIntParameter("QTY4", 1); + quantity1 = s.getParser().getIntParameter("QTY1", 0); + quantity2 = s.getParser().getIntParameter("QTY2", 0); + quantity3 = s.getParser().getIntParameter("QTY3", 0); + quantity4 = s.getParser().getIntParameter("QTY4", 0); } /* @@ -296,7 +296,7 @@ public class ConcurrencyCart extends LessonAdapter try { - Thread.sleep(5000); + //Thread.sleep(5000); ec.addElement(new HR().setWidth("90%")); ec.addElement(new Center().addElement(new H1().addElement("Thank you for your purchase!"))); @@ -430,7 +430,7 @@ public class ConcurrencyCart extends LessonAdapter tr.addElement(new TD().addElement("169.00").setAlign("right")); tr.addElement(new TD().addElement( new Input(Input.TEXT, "QTY1", s.getParser() - .getStringParameter("QTY1", "1"))) + .getStringParameter("QTY1", "0"))) .setAlign("right")); total = quantity1 * 169; @@ -444,7 +444,7 @@ public class ConcurrencyCart extends LessonAdapter tr.addElement(new TD().addElement("299.00").setAlign("right")); tr.addElement(new TD().addElement( new Input(Input.TEXT, "QTY2", s.getParser() - .getStringParameter("QTY2", "1"))) + .getStringParameter("QTY2", "0"))) .setAlign("right")); total = quantity2 * 299; @@ -458,7 +458,7 @@ public class ConcurrencyCart extends LessonAdapter tr.addElement(new TD().addElement("1799.00").setAlign("right")); tr.addElement(new TD().addElement( new Input(Input.TEXT, "QTY3", s.getParser() - .getStringParameter("QTY3", "1"))) + .getStringParameter("QTY3", "0"))) .setAlign("right")); total = quantity3 * 1799; @@ -472,7 +472,7 @@ public class ConcurrencyCart extends LessonAdapter tr.addElement(new TD().addElement("649.00").setAlign("right")); tr.addElement(new TD().addElement( new Input(Input.TEXT, "QTY4", s.getParser() - .getStringParameter("QTY4", "1"))) + .getStringParameter("QTY4", "0"))) .setAlign("right")); total = quantity4 * 649; @@ -540,7 +540,7 @@ public class ConcurrencyCart extends LessonAdapter { List hints = new ArrayList(); hints.add("Can you purchase the merchandise in your shopping cart for a lower price?"); - hints.add("Try using another web browser to get a lower price."); + hints.add("Try using a new browser window to get a lower price."); return hints; } diff --git a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/CrossSiteScripting/UpdateProfile.java b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/CrossSiteScripting/UpdateProfile.java index 55f8521d5..9dd829fea 100644 --- a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/CrossSiteScripting/UpdateProfile.java +++ b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/CrossSiteScripting/UpdateProfile.java @@ -2,6 +2,7 @@ package org.owasp.webgoat.lessons.CrossSiteScripting; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.PreparedStatement; import java.sql.Statement; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -217,6 +218,10 @@ public class UpdateProfile extends DefaultLessonAction try { // Note: The password field is ONLY set by ChangePassword + String query = "UPDATE employee SET first_name = ?, last_name = ?, ssn = ?, title = ?, phone = ?, address1 = ?, address2 = ?," + + " manager = ?, start_date = ?, ccn = ?, ccn_limit = ?," + + " personal_description = ? WHERE userid = ?;"; + /** String query = "UPDATE employee SET first_name = '" + employee.getFirstName() + "', last_name = '" + employee.getLastName() + "', ssn = '" + employee.getSsn() @@ -237,13 +242,32 @@ public class UpdateProfile extends DefaultLessonAction ", personal_description = '" + employee.getPersonalDescription() + "' WHERE userid = " + subjectId; + **/ //System.out.println("Query: " + query); try { + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + + ps.setString(1, employee.getFirstName()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getPersonalDescription()); + ps.setInt(13, subjectId); + /** Statement answer_statement = WebSession.getConnection(s) .createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - answer_statement.executeUpdate(query); + **/ + //ps.executeUpdate(query); + ps.execute(); } catch (SQLException sqle) { @@ -266,6 +290,10 @@ public class UpdateProfile extends DefaultLessonAction try { // Note: The password field is ONLY set by ChangePassword + String query = "UPDATE employee SET first_name = ?, last_name = ?, ssn = ?, title = ?, phone = ?, address1 = ?, address2 = ?," + + " manager = ?, start_date = ?, ccn = ?, ccn_limit = ?," + + " personal_description = ? WHERE userid = ?;"; + /** String query = "UPDATE employee SET first_name = '" + employee.getFirstName() + "', last_name = '" + employee.getLastName() + "', ssn = '" + employee.getSsn() @@ -286,13 +314,31 @@ public class UpdateProfile extends DefaultLessonAction ", personal_description = '" + employee.getPersonalDescription() + "' WHERE userid = " + subjectId; + **/ //System.out.println("Query: " + query); try { + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + + ps.setString(1, employee.getFirstName()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getPersonalDescription()); + ps.setInt(13, subjectId); + /** Statement answer_statement = WebSession.getConnection(s) .createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - answer_statement.executeUpdate(query); + **/ + ps.executeUpdate(query); } catch (SQLException sqle) { @@ -315,27 +361,30 @@ public class UpdateProfile extends DefaultLessonAction try { // FIXME: Cannot choose the id because we cannot guarantee uniqueness - String query = "INSERT INTO employee VALUES ( max(userid)+1, '" - + employee.getFirstName() + "','" + employee.getLastName() - + "','" + employee.getSsn() + "','" - + employee.getFirstName().toLowerCase() + "','" - + employee.getTitle() + "','" + employee.getPhoneNumber() - + "','" + employee.getAddress1() + "','" - + employee.getAddress2() + "'," + employee.getManager() - + ",'" + employee.getStartDate() + "'," - + employee.getSalary() + ",'" + employee.getCcn() + "'," - + employee.getCcnLimit() + ",'" - + employee.getDisciplinaryActionDate() + "','" - + employee.getDisciplinaryActionNotes() + "','" - + employee.getPersonalDescription() + "')"; + String query = "INSERT INTO employee VALUES ( max(userid)+1, ?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; //System.out.println("Query: " + query); try { - Statement statement = WebSession.getConnection(s) - .createStatement(); - statement.executeUpdate(query); + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query); + + ps.setString(1, employee.getFirstName().toLowerCase()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getDisciplinaryActionDate()); + ps.setString(13, employee.getDisciplinaryActionNotes()); + ps.setString(14, employee.getPersonalDescription()); + + ps.execute(); } catch (SQLException sqle) { @@ -357,27 +406,31 @@ public class UpdateProfile extends DefaultLessonAction try { // FIXME: Cannot choose the id because we cannot guarantee uniqueness - String query = "INSERT INTO employee VALUES ( max(userid)+1, '" - + employee.getFirstName() + "','" + employee.getLastName() - + "','" + employee.getSsn() + "','" - + employee.getFirstName().toLowerCase() + "','" - + employee.getTitle() + "','" + employee.getPhoneNumber() - + "','" + employee.getAddress1() + "','" - + employee.getAddress2() + "'," + employee.getManager() - + ",'" + employee.getStartDate() + "'," - + employee.getSalary() + ",'" + employee.getCcn() + "'," - + employee.getCcnLimit() + ",'" - + employee.getDisciplinaryActionDate() + "','" - + employee.getDisciplinaryActionNotes() + "','" - + employee.getPersonalDescription() + "')"; + int nextId = getNextUID(s); + String query = "INSERT INTO employee VALUES ( " + nextId + ", ?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; //System.out.println("Query: " + query); try { - Statement statement = WebSession.getConnection(s) - .createStatement(); - statement.executeUpdate(query); + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query); + + ps.setString(1, employee.getFirstName().toLowerCase()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getDisciplinaryActionDate()); + ps.setString(13, employee.getDisciplinaryActionNotes()); + ps.setString(14, employee.getPersonalDescription()); + + ps.execute(); } catch (SQLException sqle) { @@ -411,4 +464,29 @@ public class UpdateProfile extends DefaultLessonAction return parameter; } + private int getNextUID(WebSession s) + { + int uid = -1; + try + { + Statement statement = WebSession.getConnection(s).createStatement( + ResultSet.TYPE_SCROLL_INSENSITIVE, + ResultSet.CONCUR_READ_ONLY); + ResultSet results = statement + .executeQuery("select max(userid) as uid from employee"); + results.first(); + uid = results.getInt("uid"); + } + catch (SQLException sqle) + { + sqle.printStackTrace(); + s.setMessage("Error updating employee profile"); + } + catch (ClassNotFoundException e) + { + // TODO Auto-generated catch block + e.printStackTrace(); + } + return uid + 1; + } } diff --git a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ReflectedXSS.java b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ReflectedXSS.java index 3e3bb3ee8..07ffb4023 100644 --- a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ReflectedXSS.java +++ b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/ReflectedXSS.java @@ -142,7 +142,7 @@ public class ReflectedXSS extends LessonAdapter tr = new TR(); tr .addElement(new TD() - .addElement("Hewlett-Packard - Pavilion Notebook with Intel® Centrino™")); + .addElement("Hewlett-Packard - Pavilion Notebook with Intel Centrino")); tr.addElement(new TD().addElement("1599.99").setAlign("right")); tr.addElement(new TD().addElement( new Input(Input.TEXT, "QTY3", s.getParser() @@ -201,8 +201,8 @@ public class ReflectedXSS extends LessonAdapter tr = new TR(); tr.addElement(new TD() .addElement("Enter your three digit access code:")); - tr.addElement(new TD().addElement(new Input(Input.TEXT, "field1", - param1))); + tr.addElement(new TD().addElement("")); + //tr.addElement(new TD().addElement(new Input(Input.TEXT, "field1",param1))); t.addElement(tr); Element b = ECSFactory.makeButton("Purchase"); diff --git a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/RoleBasedAccessControl/UpdateProfile.java b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/RoleBasedAccessControl/UpdateProfile.java index 4dfdf2efb..f825ad3b6 100644 --- a/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/RoleBasedAccessControl/UpdateProfile.java +++ b/ webgoat/main/project/JavaSource/org/owasp/webgoat/lessons/RoleBasedAccessControl/UpdateProfile.java @@ -1,5 +1,6 @@ package org.owasp.webgoat.lessons.RoleBasedAccessControl; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -147,6 +148,10 @@ public class UpdateProfile extends DefaultLessonAction try { // Note: The password field is ONLY set by ChangePassword + String query = "UPDATE employee SET first_name = ?, last_name = ?, ssn = ?, title = ?, phone = ?, address1 = ?, address2 = ?," + + " manager = ?, start_date = ?, ccn = ?, ccn_limit = ?," + + " personal_description = ? WHERE userid = ?;"; + /** String query = "UPDATE employee SET first_name = '" + employee.getFirstName() + "', last_name = '" + employee.getLastName() + "', ssn = '" + employee.getSsn() @@ -167,13 +172,32 @@ public class UpdateProfile extends DefaultLessonAction ", personal_description = '" + employee.getPersonalDescription() + "' WHERE userid = " + subjectId; + **/ //System.out.println("Query: " + query); try { + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + + ps.setString(1, employee.getFirstName()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getPersonalDescription()); + ps.setInt(13, subjectId); + /** Statement answer_statement = WebSession.getConnection(s) .createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - answer_statement.execute(query); + **/ + //ps.executeUpdate(query); + ps.execute(); } catch (SQLException sqle) { @@ -196,6 +220,10 @@ public class UpdateProfile extends DefaultLessonAction try { // Note: The password field is ONLY set by ChangePassword + String query = "UPDATE employee SET first_name = ?, last_name = ?, ssn = ?, title = ?, phone = ?, address1 = ?, address2 = ?," + + " manager = ?, start_date = ?, ccn = ?, ccn_limit = ?," + + " personal_description = ? WHERE userid = ?;"; + /** String query = "UPDATE employee SET first_name = '" + employee.getFirstName() + "', last_name = '" + employee.getLastName() + "', ssn = '" + employee.getSsn() @@ -216,13 +244,31 @@ public class UpdateProfile extends DefaultLessonAction ", personal_description = '" + employee.getPersonalDescription() + "' WHERE userid = " + subjectId; + **/ //System.out.println("Query: " + query); try { + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); + + ps.setString(1, employee.getFirstName()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getPersonalDescription()); + ps.setInt(13, subjectId); + /** Statement answer_statement = WebSession.getConnection(s) .createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - answer_statement.execute(query); + **/ + ps.executeUpdate(query); } catch (SQLException sqle) { @@ -238,7 +284,6 @@ public class UpdateProfile extends DefaultLessonAction } } - private int getNextUID(WebSession s) { int uid = -1; @@ -265,61 +310,48 @@ public class UpdateProfile extends DefaultLessonAction return uid + 1; } - public void createEmployeeProfile(WebSession s, int userId, Employee employee) throws UnauthorizedException { try { - int newUID = getNextUID(s); - // FIXME: This max() thing doesn't work on InstantDB. - String query = "INSERT INTO employee VALUES (" + newUID + ", '" - + employee.getFirstName() + "','" + employee.getLastName() - + "','" + employee.getSsn() + "','goober57x','" - + employee.getTitle() + "','" + employee.getPhoneNumber() - + "','" + employee.getAddress1() + "','" - + employee.getAddress2() + "'," + employee.getManager() - + ",'" + employee.getStartDate() + "'," - + employee.getSalary() + ",'" + employee.getCcn() + "'," - + employee.getCcnLimit() + ",'" - + employee.getDisciplinaryActionDate() + "','" - + employee.getDisciplinaryActionNotes() + "','" - + employee.getPersonalDescription() + "')"; + // FIXME: Cannot choose the id because we cannot guarantee uniqueness + int nextId = getNextUID(s); + String query = "INSERT INTO employee VALUES ( " + nextId + ", ?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; //System.out.println("Query: " + query); try { - Statement statement = WebSession.getConnection(s) - .createStatement(); - statement.executeUpdate(query); + PreparedStatement ps = WebSession.getConnection(s).prepareStatement(query); + + ps.setString(1, employee.getFirstName().toLowerCase()); + ps.setString(2, employee.getLastName()); + ps.setString(3, employee.getSsn()); + ps.setString(4, employee.getTitle()); + ps.setString(5, employee.getPhoneNumber()); + ps.setString(6, employee.getAddress1()); + ps.setString(7, employee.getAddress2()); + ps.setInt(8, employee.getManager()); + ps.setString(9, employee.getStartDate()); + ps.setString(10, employee.getCcn()); + ps.setInt(11, employee.getCcnLimit()); + ps.setString(12, employee.getDisciplinaryActionDate()); + ps.setString(13, employee.getDisciplinaryActionNotes()); + ps.setString(14, employee.getPersonalDescription()); + + ps.execute(); } catch (SQLException sqle) { - sqle.printStackTrace(); - s.setMessage("Error updating employee profile"); - } - - query = "INSERT INTO roles VALUES (" + newUID + ", 'hr')"; - - //System.out.println("Query: " + query); - - try - { - Statement statement = WebSession.getConnection(s) - .createStatement(); - statement.executeUpdate(query); - } - catch (SQLException sqle) - { - sqle.printStackTrace(); s.setMessage("Error updating employee profile"); + sqle.printStackTrace(); } } catch (Exception e) { - e.printStackTrace(); s.setMessage("Error updating employee profile"); + e.printStackTrace(); } } } diff --git a/ webgoat/main/project/WebContent/webgoat_challenge.jsp b/ webgoat/main/project/WebContent/webgoat_challenge.jsp index 2139fce8f..fe6e840e0 100644 --- a/ webgoat/main/project/WebContent/webgoat_challenge.jsp +++ b/ webgoat/main/project/WebContent/webgoat_challenge.jsp @@ -8,20 +8,20 @@ WebGoat V5.1RC2 - + -
-
-
+
+
+

Thank you for using WebGoat!

This program is a demonstration of common web application flaws. The exercises are intended to provide hands on experience with application penetration testing techniques.

The WebGoat project is lead by Bruce Mayhew. Please send all comments to Bruce at <%= webSession.getWebgoatContext().getFeedbackAddress() %>.

-
+
@@ -83,7 +83,7 @@
OWASP Foundation
-
WARNING
+
WARNING
While running this program, your machine is extremely vulnerable to attack. You should disconnect from the network while using this program.