corrected spelling and some formatting
git-svn-id: http://webgoat.googlecode.com/svn/trunk@360 4033779f-a91e-0410-96ef-6bf7bf53c507
This commit is contained in:
parent
ba6560b24a
commit
2203a1ebd2
@ -107,10 +107,7 @@ public class DangerousEval extends LessonAdapter
|
|||||||
tr = new TR();
|
tr = new TR();
|
||||||
tr.addElement(new TD().addElement("Studio RTA - Laptop/Reading Cart with Tilting Surface - Cherry "));
|
tr.addElement(new TD().addElement("Studio RTA - Laptop/Reading Cart with Tilting Surface - Cherry "));
|
||||||
tr.addElement(new TD().addElement("69.99").setAlign("right"));
|
tr.addElement(new TD().addElement("69.99").setAlign("right"));
|
||||||
tr.addElement(new TD().addElement(
|
tr.addElement(new TD().addElement(new Input(Input.TEXT, "QTY1", s.getParser().getStringParameter("QTY1", "1"))).setAlign("right"));
|
||||||
new Input(Input.TEXT, "QTY1", s.getParser().getStringParameter("QTY1",
|
|
||||||
"1")))
|
|
||||||
.setAlign("right"));
|
|
||||||
quantity = s.getParser().getFloatParameter("QTY1", 0.0f);
|
quantity = s.getParser().getFloatParameter("QTY1", 0.0f);
|
||||||
total = quantity * 69.99f;
|
total = quantity * 69.99f;
|
||||||
runningTotal += total;
|
runningTotal += total;
|
||||||
@ -119,10 +116,7 @@ public class DangerousEval extends LessonAdapter
|
|||||||
tr = new TR();
|
tr = new TR();
|
||||||
tr.addElement(new TD().addElement("Dynex - Traditional Notebook Case"));
|
tr.addElement(new TD().addElement("Dynex - Traditional Notebook Case"));
|
||||||
tr.addElement(new TD().addElement("27.99").setAlign("right"));
|
tr.addElement(new TD().addElement("27.99").setAlign("right"));
|
||||||
tr.addElement(new TD().addElement(
|
tr.addElement(new TD().addElement(new Input(Input.TEXT, "QTY2", s.getParser().getStringParameter("QTY2", "1"))).setAlign("right"));
|
||||||
new Input(Input.TEXT, "QTY2", s.getParser().getStringParameter("QTY2",
|
|
||||||
"1")))
|
|
||||||
.setAlign("right"));
|
|
||||||
quantity = s.getParser().getFloatParameter("QTY2", 0.0f);
|
quantity = s.getParser().getFloatParameter("QTY2", 0.0f);
|
||||||
total = quantity * 27.99f;
|
total = quantity * 27.99f;
|
||||||
runningTotal += total;
|
runningTotal += total;
|
||||||
@ -131,10 +125,7 @@ public class DangerousEval extends LessonAdapter
|
|||||||
tr = new TR();
|
tr = new TR();
|
||||||
tr.addElement(new TD().addElement("Hewlett-Packard - Pavilion Notebook with Intel® Centrino™"));
|
tr.addElement(new TD().addElement("Hewlett-Packard - Pavilion Notebook with Intel® Centrino™"));
|
||||||
tr.addElement(new TD().addElement("1599.99").setAlign("right"));
|
tr.addElement(new TD().addElement("1599.99").setAlign("right"));
|
||||||
tr.addElement(new TD().addElement(
|
tr.addElement(new TD().addElement(new Input(Input.TEXT, "QTY3", s.getParser().getStringParameter("QTY3", "1"))).setAlign("right"));
|
||||||
new Input(Input.TEXT, "QTY3", s.getParser().getStringParameter("QTY3",
|
|
||||||
"1")))
|
|
||||||
.setAlign("right"));
|
|
||||||
quantity = s.getParser().getFloatParameter("QTY3", 0.0f);
|
quantity = s.getParser().getFloatParameter("QTY3", 0.0f);
|
||||||
total = quantity * 1599.99f;
|
total = quantity * 1599.99f;
|
||||||
runningTotal += total;
|
runningTotal += total;
|
||||||
@ -144,10 +135,7 @@ public class DangerousEval extends LessonAdapter
|
|||||||
tr.addElement(new TD().addElement("3 - Year Performance Service Plan $1000 and Over "));
|
tr.addElement(new TD().addElement("3 - Year Performance Service Plan $1000 and Over "));
|
||||||
tr.addElement(new TD().addElement("299.99").setAlign("right"));
|
tr.addElement(new TD().addElement("299.99").setAlign("right"));
|
||||||
|
|
||||||
tr.addElement(new TD().addElement(
|
tr.addElement(new TD().addElement(new Input(Input.TEXT, "QTY4", s.getParser().getStringParameter("QTY4", "1"))).setAlign("right"));
|
||||||
new Input(Input.TEXT, "QTY4", s.getParser().getStringParameter("QTY4",
|
|
||||||
"1")))
|
|
||||||
.setAlign("right"));
|
|
||||||
quantity = s.getParser().getFloatParameter("QTY4", 0.0f);
|
quantity = s.getParser().getFloatParameter("QTY4", 0.0f);
|
||||||
total = quantity * 299.99f;
|
total = quantity * 299.99f;
|
||||||
runningTotal += total;
|
runningTotal += total;
|
||||||
@ -232,16 +220,15 @@ public class DangerousEval extends LessonAdapter
|
|||||||
hints.add("The lesson is similar to the standard reflected cross-site scripting lesson.");
|
hints.add("The lesson is similar to the standard reflected cross-site scripting lesson.");
|
||||||
hints.add("The access code parameter is vulnerable to a reflected cross-site scripting problem.");
|
hints.add("The access code parameter is vulnerable to a reflected cross-site scripting problem.");
|
||||||
hints.add("The usual <SCRIPT>alert(document.cookie);</SCRIPT> will not work in this lesson. Why?");
|
hints.add("The usual <SCRIPT>alert(document.cookie);</SCRIPT> will not work in this lesson. Why?");
|
||||||
hints
|
hints.add("User-supplied data is landing in the Javascript eval() function. Your attack will not require the < and > characters.");
|
||||||
.add("User-supplied data is landing in the Javascript eval() function. Your attack will not require the < and > characters.");
|
|
||||||
hints.add("In order to pass this lesson, you must 'alert' the document.cookie.");
|
hints.add("In order to pass this lesson, you must 'alert' the document.cookie.");
|
||||||
hints.add("Try 123');alert(document.cookie);('");
|
hints.add("Try 123');alert(document.cookie);('");
|
||||||
return hints;
|
return hints;
|
||||||
}
|
}
|
||||||
|
|
||||||
// <script type="text/javascript">if ( navigator.appName.indexOf("Microsoft") !=-1) {var xmlHttp
|
|
||||||
// = new
|
// <script type="text/javascript">if ( navigator.appName.indexOf("Microsoft") !=-1)
|
||||||
// ActiveXObject("Microsoft.XMLHTTP");xmlHttp.open("TRACE", "./", false);
|
// {var xmlHttp = new ActiveXObject("Microsoft.XMLHTTP");xmlHttp.open("TRACE", "./", false);
|
||||||
// xmlHttp.send();str1=xmlHttp.responseText;document.write(str1);}</script>
|
// xmlHttp.send();str1=xmlHttp.responseText;document.write(str1);}</script>
|
||||||
/**
|
/**
|
||||||
* Gets the instructions attribute of the WeakAccessControl object
|
* Gets the instructions attribute of the WeakAccessControl object
|
||||||
|
@ -31,6 +31,7 @@ import org.owasp.webgoat.session.WebSession;
|
|||||||
* License, or (at your option) any later version.
|
* License, or (at your option) any later version.
|
||||||
*
|
*
|
||||||
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
|
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
|
||||||
|
* even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
||||||
* General Public License for more details.
|
* General Public License for more details.
|
||||||
*
|
*
|
||||||
* You should have received a copy of the GNU General Public License along with this program; if
|
* You should have received a copy of the GNU General Public License along with this program; if
|
||||||
|
@ -345,8 +345,7 @@ public class RoleBasedAccessControl extends GoatHillsFinancial
|
|||||||
if (action != null)
|
if (action != null)
|
||||||
{
|
{
|
||||||
// System.out.println("RoleBasedAccessControl.handleRequest() dispatching to: "
|
// System.out.println("RoleBasedAccessControl.handleRequest() dispatching to: "
|
||||||
// +
|
// + action.getActionName());
|
||||||
// action.getActionName());
|
|
||||||
if (!action.requiresAuthentication())
|
if (!action.requiresAuthentication())
|
||||||
{
|
{
|
||||||
// Access to Login does not require authentication.
|
// Access to Login does not require authentication.
|
||||||
|
@ -114,7 +114,7 @@ public class SqlStringInjection extends SequentialLessonAdapter
|
|||||||
StringBuffer msg = new StringBuffer();
|
StringBuffer msg = new StringBuffer();
|
||||||
|
|
||||||
msg.append("Bet you can't do it again! ");
|
msg.append("Bet you can't do it again! ");
|
||||||
msg.append("This lesson has detected your successfull attack ");
|
msg.append("This lesson has detected your successful attack ");
|
||||||
msg.append("and has now switched to a defensive mode. ");
|
msg.append("and has now switched to a defensive mode. ");
|
||||||
msg.append("Try again to attack a parameterized query.");
|
msg.append("Try again to attack a parameterized query.");
|
||||||
|
|
||||||
|
@ -234,8 +234,7 @@ public class TraceXSS extends LessonAdapter
|
|||||||
}
|
}
|
||||||
|
|
||||||
// <script type="text/javascript">if ( navigator.appName.indexOf("Microsoft") !=-1) {var xmlHttp
|
// <script type="text/javascript">if ( navigator.appName.indexOf("Microsoft") !=-1) {var xmlHttp
|
||||||
// = new
|
// = new ActiveXObject("Microsoft.XMLHTTP");xmlHttp.open("TRACE", "./", false);
|
||||||
// ActiveXObject("Microsoft.XMLHTTP");xmlHttp.open("TRACE", "./", false);
|
|
||||||
// xmlHttp.send();str1=xmlHttp.responseText;document.write(str1);}</script>
|
// xmlHttp.send();str1=xmlHttp.responseText;document.write(str1);}</script>
|
||||||
|
|
||||||
private final static Integer DEFAULT_RANKING = new Integer(130);
|
private final static Integer DEFAULT_RANKING = new Integer(130);
|
||||||
|
@ -268,9 +268,8 @@ public class ReportCardScreen extends LessonAdapter
|
|||||||
{
|
{
|
||||||
H2 h2 = new H2();
|
H2 h2 = new H2();
|
||||||
// FIXME: The session is the current session, not the session of the user we are reporting.
|
// FIXME: The session is the current session, not the session of the user we are reporting.
|
||||||
// String type = s.isAdmin() ? " [Administrative User]" : s.isHackedAdmin() ? " [Normal User
|
// String type = s.isAdmin() ? " [Administrative User]" : s.isHackedAdmin() ?
|
||||||
// - Hacked Admin
|
// " [Normal User - Hacked Admin Access]" : " [Normal User]";
|
||||||
// Access]" : " [Normal User]";
|
|
||||||
String type = "";
|
String type = "";
|
||||||
h2.addElement(new StringElement("Results for: " + user + type));
|
h2.addElement(new StringElement("Results for: " + user + type));
|
||||||
return h2;
|
return h2;
|
||||||
|
@ -1,4 +1,3 @@
|
|||||||
|
|
||||||
package org.owasp.webgoat.lessons.instructor.CrossSiteScripting;
|
package org.owasp.webgoat.lessons.instructor.CrossSiteScripting;
|
||||||
|
|
||||||
import java.util.regex.Pattern;
|
import java.util.regex.Pattern;
|
||||||
@ -10,15 +9,25 @@ import org.owasp.webgoat.session.ValidationException;
|
|||||||
import org.owasp.webgoat.session.WebSession;
|
import org.owasp.webgoat.session.WebSession;
|
||||||
|
|
||||||
|
|
||||||
/*
|
// STAGE 5 FIXES
|
||||||
* STAGE 5 FIXES Solution Summary: Edit FindProfile.java and change getRequestParameter(). Modify
|
// Solution Summary: Edit FindProfile.java and change getRequestParameter().
|
||||||
* getRequestParameter() with lines denoted by // STAGE 5 - FIX. Solution Steps: 1. Talk about the
|
// Modify getRequestParameter() with lines denoted by // STAGE 5 - FIX.
|
||||||
* different parser methods. We could have used the parser method that takes a regular expression.
|
// Solution Steps:
|
||||||
* 2. Call validate on the request parameter. return validate(s.getParser().getRawParameter(name),
|
// 1. Talk about the different parser methods. We could have used the parser method that takes a
|
||||||
* (Pattern) patterns.get(name)); Note: patterns.get(name) is used to fetch the XSS validation
|
// regular expression.
|
||||||
* pattern that is defined in FindProfile.Java protected static Map patterns = new HashMap(); static
|
// 2. Call validate on the request parameter.
|
||||||
* { patterns.put(CrossSiteScripting.SEARCHNAME, Pattern.compile("[a-zA-Z ]{0,20}")); }
|
// return validate(s.getParser().getRawParameter(name), (Pattern) patterns.get(name));
|
||||||
*/
|
//
|
||||||
|
// Note: patterns.get(name) is used to fetch the XSS validation pattern that is defined
|
||||||
|
// in FindProfile.Java
|
||||||
|
//
|
||||||
|
// protected static Map patterns = new HashMap();
|
||||||
|
// static
|
||||||
|
// {
|
||||||
|
// patterns.put(CrossSiteScripting.SEARCHNAME, Pattern.compile("[a-zA-Z ]{0,20}"));
|
||||||
|
// }
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
public class FindProfile_i extends FindProfile
|
public class FindProfile_i extends FindProfile
|
||||||
{
|
{
|
||||||
|
@ -14,19 +14,31 @@ import org.owasp.webgoat.session.ValidationException;
|
|||||||
import org.owasp.webgoat.session.WebSession;
|
import org.owasp.webgoat.session.WebSession;
|
||||||
|
|
||||||
|
|
||||||
/*
|
// STAGE 2 FIXES
|
||||||
* STAGE 2 FIXES Solution Summary: Edit UpdateProfile.java and change parseEmployeeProfile(). Modify
|
// Solution Summary: Edit UpdateProfile.java and change parseEmployeeProfile().
|
||||||
* parseEmployeeProfile() with lines denoted by // STAGE 2 - FIX. Solution Steps: 1. Talk about the
|
// Modify parseEmployeeProfile() with lines denoted by // STAGE 2 - FIX.
|
||||||
* different parser methods. a. parseEmployeeProfile(subjectId, s.getRequest()) - uses the request
|
// Solution Steps:
|
||||||
* object directly. - calling validate() on the appropriate parameter b.
|
// 1. Talk about the different parser methods.
|
||||||
* parseEmployeeProfile(subjectId, s.getParser()) - uses the parser object to pull request data
|
// a. parseEmployeeProfile(subjectId, s.getRequest())
|
||||||
* (centralized mechanism) 2. Fix the request object version of the call // STAGE 2 - FIX Replace
|
// - uses the request object directly.
|
||||||
* the call to: String address1 = request.getParameter(CrossSiteScripting.ADDRESS1); With: final
|
// - calling validate() on the appropriate parameter
|
||||||
* Pattern PATTERN_ADDRESS1 = Pattern.compile("[a-zA-Z0-9,\\.\\- ]{0,80}"); // STAGE 2 - FIX String
|
// b. parseEmployeeProfile(subjectId, s.getParser())
|
||||||
* address1 = validate(request.getParameter(CrossSiteScripting.ADDRESS1), PATTERN_ADDRESS1); //
|
// - uses the parser object to pull request data (centralized mechanism)
|
||||||
* STAGE 2 - FIX 3. Fix the parser version of the call. // STAGE 2 - ALTERNATE FIX Change all calls
|
//
|
||||||
* in parseEmployeeProfile(subjectId, s.getParser()) to use the appropriate parser.method() call
|
// 2. Fix the request object version of the call // STAGE 2 - FIX
|
||||||
*/
|
// Replace the call to:
|
||||||
|
// String address1 = request.getParameter(CrossSiteScripting.ADDRESS1);
|
||||||
|
//
|
||||||
|
// With:
|
||||||
|
// final Pattern PATTERN_ADDRESS1 = Pattern.compile("[a-zA-Z0-9,\\.\\- ]{0,80}"); // STAGE 2 - FIX
|
||||||
|
// String address1 = validate(request.getParameter(CrossSiteScripting.ADDRESS1), PATTERN_ADDRESS1);
|
||||||
|
// // STAGE 2 - FIX
|
||||||
|
//
|
||||||
|
//
|
||||||
|
// 3. Fix the parser version of the call. // STAGE 2 - ALTERNATE FIX
|
||||||
|
// Change all calls in parseEmployeeProfile(subjectId, s.getParser()) to use
|
||||||
|
// the appropriate parser.method() call
|
||||||
|
//
|
||||||
|
|
||||||
public class UpdateProfile_i extends UpdateProfile
|
public class UpdateProfile_i extends UpdateProfile
|
||||||
{
|
{
|
||||||
@ -47,12 +59,11 @@ public class UpdateProfile_i extends UpdateProfile
|
|||||||
|
|
||||||
// Validate this parameter against a regular expression pattern designed for street
|
// Validate this parameter against a regular expression pattern designed for street
|
||||||
// addresses.
|
// addresses.
|
||||||
final Pattern PATTERN_ADDRESS1 = Pattern.compile("[a-zA-Z0-9,\\.\\- ]{0,80}"); // STAGE 2 -
|
|
||||||
// FIX
|
// STAGE 2 - FIX
|
||||||
String address1 = validate(request.getParameter(CrossSiteScripting.ADDRESS1), PATTERN_ADDRESS1); // STAGE
|
final Pattern PATTERN_ADDRESS1 = Pattern.compile("[a-zA-Z0-9,\\.\\- ]{0,80}");
|
||||||
// 2
|
String address1 = validate(request.getParameter(CrossSiteScripting.ADDRESS1), PATTERN_ADDRESS1);
|
||||||
// -
|
|
||||||
// FIX
|
|
||||||
|
|
||||||
String address2 = request.getParameter(CrossSiteScripting.ADDRESS2);
|
String address2 = request.getParameter(CrossSiteScripting.ADDRESS2);
|
||||||
int manager = Integer.parseInt(request.getParameter(CrossSiteScripting.MANAGER));
|
int manager = Integer.parseInt(request.getParameter(CrossSiteScripting.MANAGER));
|
||||||
|
@ -5,10 +5,13 @@ import org.owasp.webgoat.lessons.CrossSiteScripting.ViewProfile;
|
|||||||
import org.owasp.webgoat.lessons.GoatHillsFinancial.GoatHillsFinancial;
|
import org.owasp.webgoat.lessons.GoatHillsFinancial.GoatHillsFinancial;
|
||||||
|
|
||||||
|
|
||||||
/*
|
// STAGE 4 FIXES
|
||||||
* STAGE 4 FIXES Solution Summary: Look in the WebContent/lesson/CrossSiteScripting/ViewProfile.jsp
|
//
|
||||||
* Look for the <-- STAGE 4 - FIX in the ViewProfile.jsp
|
//Solution Summary: Look in the WebContent/lesson/CrossSiteScripting/ViewProfile.jsp
|
||||||
*/
|
//
|
||||||
|
//Look for the <-- STAGE 4 - FIX in the ViewProfile.jsp
|
||||||
|
//
|
||||||
|
//
|
||||||
|
|
||||||
public class ViewProfile_i extends ViewProfile
|
public class ViewProfile_i extends ViewProfile
|
||||||
{
|
{
|
||||||
|
@ -5,55 +5,128 @@ import org.owasp.webgoat.lessons.GoatHillsFinancial.LessonAction;
|
|||||||
import org.owasp.webgoat.lessons.CrossSiteScripting.UpdateProfile;
|
import org.owasp.webgoat.lessons.CrossSiteScripting.UpdateProfile;
|
||||||
import org.owasp.webgoat.lessons.GoatHillsFinancial.GoatHillsFinancial;
|
import org.owasp.webgoat.lessons.GoatHillsFinancial.GoatHillsFinancial;
|
||||||
|
|
||||||
|
/* STAGE 2 FIXES
|
||||||
|
Solution Summary (1. or 2.)
|
||||||
|
1. Modify the UPDATE_EMPLOYEE stored procedure in the database and add
|
||||||
|
a validation step. Oracle 10G now supports regular expressions.
|
||||||
|
2. Apply a column constraint can also work IFF the existing data is clean
|
||||||
|
|
||||||
/*
|
Solution Steps:
|
||||||
* STAGE 2 FIXES Solution Summary (1. or 2.) 1. Modify the UPDATE_EMPLOYEE stored procedure in the
|
1. Talk about the different database approaches.
|
||||||
* database and add a validation step. Oracle 10G now supports regular expressions. 2. Apply a
|
a. Apply validation in the UPDATE stored proc
|
||||||
* column constraint can also work IFF the existing data is clean Solution Steps: 1. Talk about the
|
- Possible to bypass by not using that stored proc
|
||||||
* different database approaches. a. Apply validation in the UPDATE stored proc - Possible to bypass
|
|
||||||
* by not using that stored proc b. Apply a table column constraint - Cannot be bypassed. The DB
|
b. Apply a table column constraint
|
||||||
* enforces the constraint under all conditions 2. Fix the stored proc Define the pattern. Validate
|
- Cannot be bypassed. The DB enforces the constraint under all conditions
|
||||||
* the field against the pattern. Raise an exception if invalid. CREATE OR REPLACE PROCEDURE
|
|
||||||
* UPDATE_EMPLOYEE( v_userid IN employee.userid%type, v_first_name IN employee.first_name%type,
|
2. Fix the stored proc
|
||||||
* v_last_name IN employee.last_name%type, v_ssn IN employee.ssn%type, v_title IN
|
|
||||||
* employee.title%type, v_phone IN employee.phone%type, v_address1 IN employee.address1%type,
|
Define the pattern.
|
||||||
* v_address2 IN employee.address2%type, v_manager IN employee.manager%type, v_start_date IN
|
Validate the field against the pattern.
|
||||||
* employee.start_date%type, v_salary IN employee.salary%type, v_ccn IN employee.ccn%type,
|
Raise an exception if invalid.
|
||||||
* v_ccn_limit IN employee.ccn_limit%type, v_disciplined_date IN employee.disciplined_date%type,
|
|
||||||
* v_disciplined_notes IN employee.disciplined_notes%type, v_personal_description IN
|
CREATE OR REPLACE PROCEDURE UPDATE_EMPLOYEE(
|
||||||
* employee.personal_description%type ) AS P_ADDRESS1 VARCHAR2(100) := '^[a-zA-Z0-9,\. ]{0,80}$';
|
v_userid IN employee.userid%type,
|
||||||
* BEGIN IF NOT REGEXP_LIKE(v_address1, P_ADDRESS1) THEN RAISE VALUE_ERROR; END IF; UPDATE EMPLOYEE
|
v_first_name IN employee.first_name%type,
|
||||||
* SET first_name = v_first_name, last_name = v_last_name, ssn = v_ssn, title = v_title, phone =
|
v_last_name IN employee.last_name%type,
|
||||||
* v_phone, address1 = v_address1, address2 = v_address2, manager = v_manager, start_date =
|
v_ssn IN employee.ssn%type,
|
||||||
* v_Start_date, salary = v_salary, ccn = v_ccn, ccn_limit = v_ccn_limit, disciplined_date =
|
v_title IN employee.title%type,
|
||||||
* v_disciplined_date, disciplined_notes = v_disciplined_notes, personal_description =
|
v_phone IN employee.phone%type,
|
||||||
* v_personal_description WHERE userid = v_userid; END; / 3. Apply a table column constraint ALTER
|
v_address1 IN employee.address1%type,
|
||||||
* TABLE EMPLOYEE ADD CONSTRAINT address1_ck CHECK (REGEXP_LIKE(address1, '^[a-zA-Z0-9,\.
|
v_address2 IN employee.address2%type,
|
||||||
* ]{0,80}$')); FOR SQL SERVER, the following is required: DROP PROCEDURE
|
v_manager IN employee.manager%type,
|
||||||
* webgoat_guest.UPDATE_EMPLOYEE GO CREATE PROCEDURE webgoat_guest.UPDATE_EMPLOYEE
|
v_start_date IN employee.start_date%type,
|
||||||
* @v_userid INT,
|
v_salary IN employee.salary%type,
|
||||||
* @v_first_name VARCHAR(20),
|
v_ccn IN employee.ccn%type,
|
||||||
* @v_last_name VARCHAR(20),
|
v_ccn_limit IN employee.ccn_limit%type,
|
||||||
* @v_ssn VARCHAR(12),
|
v_disciplined_date IN employee.disciplined_date%type,
|
||||||
* @v_title VARCHAR(20),
|
v_disciplined_notes IN employee.disciplined_notes%type,
|
||||||
* @v_phone VARCHAR(13),
|
v_personal_description IN employee.personal_description%type
|
||||||
* @v_address1 VARCHAR(80),
|
)
|
||||||
* @v_address2 VARCHAR(80),
|
AS
|
||||||
* @v_manager INT,
|
P_ADDRESS1 VARCHAR2(100) := '^[a-zA-Z0-9,\. ]{0,80}$';
|
||||||
* @v_start_date CHAR(8),
|
BEGIN
|
||||||
* @v_salary INT,
|
IF NOT REGEXP_LIKE(v_address1, P_ADDRESS1) THEN
|
||||||
* @v_ccn VARCHAR(30),
|
RAISE VALUE_ERROR;
|
||||||
* @v_ccn_limit INT,
|
END IF;
|
||||||
* @v_disciplined_date CHAR(8),
|
UPDATE EMPLOYEE
|
||||||
* @v_disciplined_notes VARCHAR(60),
|
SET
|
||||||
* @v_personal_description VARCHAR(60) AS IF [webgoat_guest].RegexMatch(@v_address1,
|
first_name = v_first_name,
|
||||||
* N'^[a-zA-Z0-9,\. ]{0,80}$') = 0 BEGIN RAISERROR('Illegal characters in address1', 11, 1) RETURN
|
last_name = v_last_name,
|
||||||
* END UPDATE EMPLOYEE SET first_name = @v_first_name, last_name = @v_last_name, ssn = @v_ssn, title
|
ssn = v_ssn,
|
||||||
* = @v_title, phone = @v_phone, address1 = @v_address1, address2 = @v_address2, manager =
|
title = v_title,
|
||||||
* @v_manager, start_date = @v_Start_date, salary = @v_salary, ccn = @v_ccn, ccn_limit =
|
phone = v_phone,
|
||||||
* @v_ccn_limit, disciplined_date = @v_disciplined_date, disciplined_notes = @v_disciplined_notes,
|
address1 = v_address1,
|
||||||
* personal_description = @v_personal_description WHERE userid = @v_userid; GO
|
address2 = v_address2,
|
||||||
*/
|
manager = v_manager,
|
||||||
|
start_date = v_Start_date,
|
||||||
|
salary = v_salary,
|
||||||
|
ccn = v_ccn,
|
||||||
|
ccn_limit = v_ccn_limit,
|
||||||
|
disciplined_date = v_disciplined_date,
|
||||||
|
disciplined_notes = v_disciplined_notes,
|
||||||
|
personal_description = v_personal_description
|
||||||
|
WHERE
|
||||||
|
userid = v_userid;
|
||||||
|
END;
|
||||||
|
/
|
||||||
|
|
||||||
|
3. Apply a table column constraint
|
||||||
|
ALTER TABLE EMPLOYEE
|
||||||
|
ADD CONSTRAINT address1_ck CHECK (REGEXP_LIKE(address1, '^[a-zA-Z0-9,\. ]{0,80}$'));
|
||||||
|
|
||||||
|
|
||||||
|
FOR SQL SERVER, the following is required:
|
||||||
|
|
||||||
|
|
||||||
|
DROP PROCEDURE webgoat_guest.UPDATE_EMPLOYEE
|
||||||
|
GO
|
||||||
|
|
||||||
|
CREATE PROCEDURE webgoat_guest.UPDATE_EMPLOYEE
|
||||||
|
@v_userid INT,
|
||||||
|
@v_first_name VARCHAR(20),
|
||||||
|
@v_last_name VARCHAR(20),
|
||||||
|
@v_ssn VARCHAR(12),
|
||||||
|
@v_title VARCHAR(20),
|
||||||
|
@v_phone VARCHAR(13),
|
||||||
|
@v_address1 VARCHAR(80),
|
||||||
|
@v_address2 VARCHAR(80),
|
||||||
|
@v_manager INT,
|
||||||
|
@v_start_date CHAR(8),
|
||||||
|
@v_salary INT,
|
||||||
|
@v_ccn VARCHAR(30),
|
||||||
|
@v_ccn_limit INT,
|
||||||
|
@v_disciplined_date CHAR(8),
|
||||||
|
@v_disciplined_notes VARCHAR(60),
|
||||||
|
@v_personal_description VARCHAR(60)
|
||||||
|
AS
|
||||||
|
IF [webgoat_guest].RegexMatch(@v_address1, N'^[a-zA-Z0-9,\. ]{0,80}$') = 0
|
||||||
|
BEGIN
|
||||||
|
RAISERROR('Illegal characters in address1', 11, 1)
|
||||||
|
RETURN
|
||||||
|
END
|
||||||
|
UPDATE EMPLOYEE
|
||||||
|
SET
|
||||||
|
first_name = @v_first_name,
|
||||||
|
last_name = @v_last_name,
|
||||||
|
ssn = @v_ssn,
|
||||||
|
title = @v_title,
|
||||||
|
phone = @v_phone,
|
||||||
|
address1 = @v_address1,
|
||||||
|
address2 = @v_address2,
|
||||||
|
manager = @v_manager,
|
||||||
|
start_date = @v_Start_date,
|
||||||
|
salary = @v_salary,
|
||||||
|
ccn = @v_ccn,
|
||||||
|
ccn_limit = @v_ccn_limit,
|
||||||
|
disciplined_date = @v_disciplined_date,
|
||||||
|
disciplined_notes = @v_disciplined_notes,
|
||||||
|
personal_description = @v_personal_description
|
||||||
|
WHERE
|
||||||
|
userid = @v_userid;
|
||||||
|
GO
|
||||||
|
|
||||||
|
*/
|
||||||
|
|
||||||
public class UpdateProfile_i extends UpdateProfile
|
public class UpdateProfile_i extends UpdateProfile
|
||||||
{
|
{
|
||||||
|
@ -2,16 +2,44 @@
|
|||||||
package org.owasp.webgoat.lessons.instructor.DBSQLInjection;
|
package org.owasp.webgoat.lessons.instructor.DBSQLInjection;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* The solution is to choose Neville's userid, and enter a password like: ' OR '1'='1 Modify the
|
* The solution is to choose Neville's userid, and enter a password like:
|
||||||
* Stored function LOGIN_EMPLOYEE to use fixed statements or bind variables For ORACLE: CREATE OR
|
* ' OR '1'='1
|
||||||
* REPLACE FUNCTION WEBGOAT_guest.EMPLOYEE_LOGIN(v_id NUMBER, v_password VARCHAR) RETURN NUMBER AS
|
* Modify the Stored function LOGIN_EMPLOYEE to use fixed statements or bind variables
|
||||||
* cnt NUMBER; BEGIN SELECT COUNT() INTO cnt FROM EMPLOYEE WHERE USERID = v_id AND PASSWORD =
|
*
|
||||||
* v_password; RETURN cnt; END; / OR CREATE OR REPLACE FUNCTION WEBGOAT_guest.EMPLOYEE_LOGIN(v_id
|
*
|
||||||
* NUMBER, v_password VARCHAR) RETURN NUMBER AS stmt VARCHAR(32767); cnt NUMBER; BEGIN stmt :=
|
* For ORACLE:
|
||||||
* 'SELECT COUNT () FROM EMPLOYEE WHERE USERID = :1 AND PASSWORD = :2'; EXECUTE IMMEDIATE stmt INTO
|
CREATE OR REPLACE FUNCTION WEBGOAT_guest.EMPLOYEE_LOGIN(v_id NUMBER, v_password VARCHAR) RETURN NUMBER AS
|
||||||
* cnt USING v_id, v_password; RETURN cnt; END; / For SQL SERVER CREATE FUNCTION
|
cnt NUMBER;
|
||||||
* webgoat_guest.EMPLOYEE_LOGIN (
|
BEGIN
|
||||||
* @v_id INT,
|
SELECT COUNT(*) INTO cnt FROM EMPLOYEE
|
||||||
* @v_password VARCHAR(100) ) RETURNS INTEGER AS BEGIN DECLARE @count int SELECT @count = COUNT()
|
WHERE USERID = v_id
|
||||||
* FROM EMPLOYEE WHERE USERID = @v_id AND PASSWORD = @v_password; return @count END
|
AND PASSWORD = v_password;
|
||||||
*/
|
RETURN cnt;
|
||||||
|
END;
|
||||||
|
/
|
||||||
|
|
||||||
|
* OR
|
||||||
|
|
||||||
|
CREATE OR REPLACE FUNCTION WEBGOAT_guest.EMPLOYEE_LOGIN(v_id NUMBER, v_password VARCHAR) RETURN NUMBER AS
|
||||||
|
stmt VARCHAR(32767); cnt NUMBER;
|
||||||
|
BEGIN
|
||||||
|
stmt := 'SELECT COUNT (*) FROM EMPLOYEE WHERE USERID = :1 AND PASSWORD = :2';
|
||||||
|
EXECUTE IMMEDIATE stmt INTO cnt USING v_id, v_password;
|
||||||
|
RETURN cnt;
|
||||||
|
END;
|
||||||
|
/
|
||||||
|
|
||||||
|
* For SQL SERVER
|
||||||
|
|
||||||
|
CREATE FUNCTION webgoat_guest.EMPLOYEE_LOGIN (
|
||||||
|
@v_id INT,
|
||||||
|
@v_password VARCHAR(100)
|
||||||
|
) RETURNS INTEGER
|
||||||
|
AS
|
||||||
|
BEGIN
|
||||||
|
DECLARE @count int
|
||||||
|
SELECT @count = COUNT(*) FROM EMPLOYEE WHERE USERID = @v_id AND PASSWORD = @v_password;
|
||||||
|
return @count
|
||||||
|
END
|
||||||
|
|
||||||
|
*/
|
||||||
|
@ -57,15 +57,23 @@ public class EditProfile_i extends EditProfile
|
|||||||
if (answer_results.next())
|
if (answer_results.next())
|
||||||
{
|
{
|
||||||
// Note: Do NOT get the password field.
|
// Note: Do NOT get the password field.
|
||||||
profile = new Employee(answer_results.getInt("userid"), answer_results.getString("first_name"),
|
profile = new Employee(
|
||||||
answer_results.getString("last_name"), answer_results.getString("ssn"), answer_results
|
answer_results.getInt("userid"),
|
||||||
.getString("title"), answer_results.getString("phone"), answer_results
|
answer_results.getString("first_name"),
|
||||||
.getString("address1"), answer_results.getString("address2"), answer_results
|
answer_results.getString("last_name"),
|
||||||
.getInt("manager"), answer_results.getString("start_date"), answer_results
|
answer_results.getString("ssn"),
|
||||||
.getInt("salary"), answer_results.getString("ccn"), answer_results
|
answer_results.getString("title"),
|
||||||
.getInt("ccn_limit"), answer_results.getString("disciplined_date"),
|
answer_results.getString("phone"),
|
||||||
answer_results.getString("disciplined_notes"), answer_results
|
answer_results.getString("address1"),
|
||||||
.getString("personal_description"));
|
answer_results.getString("address2"),
|
||||||
|
answer_results.getInt("manager"),
|
||||||
|
answer_results.getString("start_date"),
|
||||||
|
answer_results.getInt("salary"),
|
||||||
|
answer_results.getString("ccn"),
|
||||||
|
answer_results.getInt("ccn_limit"),
|
||||||
|
answer_results.getString("disciplined_date"),
|
||||||
|
answer_results.getString("disciplined_notes"),
|
||||||
|
answer_results.getString("personal_description"));
|
||||||
/*
|
/*
|
||||||
* System.out.println("Retrieved employee from db: " +
|
* System.out.println("Retrieved employee from db: " +
|
||||||
* profile.getFirstName() + " " + profile.getLastName() + " (" +
|
* profile.getFirstName() + " " + profile.getLastName() + " (" +
|
||||||
|
@ -21,25 +21,34 @@ import org.owasp.webgoat.session.WebSession;
|
|||||||
*
|
*
|
||||||
*/
|
*/
|
||||||
|
|
||||||
/*
|
/* STAGE 2 FIXES
|
||||||
* STAGE 2 FIXES Solution Summary: Edit RoleBasedAccessControl.java and change handleRequest().
|
Solution Summary: Edit RoleBasedAccessControl.java and change handleRequest().
|
||||||
* Modify handleRequest() with lines denoted by // STAGE 2 - FIX. Solution Steps: 1. This solution
|
Modify handleRequest() with lines denoted by // STAGE 2 - FIX.
|
||||||
* adds an access control check in the controller. Point out that their architecture may require the
|
Solution Steps:
|
||||||
* check to occur in the business function. 2. Look at the RoleBasedAccessControl class identify
|
1. This solution adds an access control check in the controller.
|
||||||
* where execution happens of an action. a. action.handleRequest(s); is not protected by an access
|
Point out that their architecture may require the check to occur in the business function.
|
||||||
* control check. b. look at handleRequest(s) to determine where access control check should occur.
|
2. Look at the RoleBasedAccessControl class identify where execution happens of an action.
|
||||||
* c. add protection by a programmatic authorization check before dispatching to the action: 1. Add
|
a. action.handleRequest(s); is not protected by an access control check.
|
||||||
* an isAuthorized() call before dispatching to the action, and throw an unauthorized exception.
|
b. look at handleRequest(s) to determine where access control check should occur.
|
||||||
* Tell student this exception exists. Use eclipse command completion to find the isAuthorized()
|
c. add protection by a programmatic authorization check before dispatching to the action:
|
||||||
* call on the action. From command completion - determine calling arguments of isAuthorized() int
|
1. Add an isAuthorized() call before dispatching to the action,
|
||||||
* userId = action.getUserId(s); if (action.isAuthorized(s, userId, action.getActionName())) {
|
and throw an unauthorized exception. Tell student this exception exists.
|
||||||
* action.handleRequest(s); } else throw new UnauthorizedException(); Repeat stage 1 and note that
|
Use eclipse command completion to find the isAuthorized() call on the action.
|
||||||
* the function fails with a "Not authorized" message. Tom will be in the list again, because the DB
|
From command completion - determine calling arguments of isAuthorized()
|
||||||
* is reset when lesson restarts. Adding the access check in the
|
|
||||||
* RoleBasedAccessControl:handleRequest() is putting the check in the “Controller” The access check
|
int userId = action.getUserId(s);
|
||||||
* can also be added to DeleteProfile.deleteEmployeeProfile(), which is putting the check in the
|
if (action.isAuthorized(s, userId, action.getActionName()))
|
||||||
* “Business Function”
|
{
|
||||||
*/
|
action.handleRequest(s);
|
||||||
|
}
|
||||||
|
else
|
||||||
|
throw new UnauthorizedException();
|
||||||
|
|
||||||
|
Repeat stage 1 and note that the function fails with a "Not authorized" message.
|
||||||
|
Tom will be in the list again, because the DB is reset when lesson restarts.
|
||||||
|
Adding the access check in the RoleBasedAccessControl:handleRequest() is putting the check in the “Controller”
|
||||||
|
The access check can also be added to DeleteProfile.deleteEmployeeProfile(), which is putting the check in the “Business Function”
|
||||||
|
*/
|
||||||
|
|
||||||
public class RoleBasedAccessControl_i extends RoleBasedAccessControl
|
public class RoleBasedAccessControl_i extends RoleBasedAccessControl
|
||||||
{
|
{
|
||||||
|
@ -10,32 +10,40 @@ import org.owasp.webgoat.session.Employee;
|
|||||||
import org.owasp.webgoat.session.UnauthorizedException;
|
import org.owasp.webgoat.session.UnauthorizedException;
|
||||||
import org.owasp.webgoat.session.WebSession;
|
import org.owasp.webgoat.session.WebSession;
|
||||||
|
|
||||||
|
/* STAGE 4 FIXES
|
||||||
/*
|
1. Find the code location where this flaw of directly retrieving the profile without data-level access control checking exists:
|
||||||
* STAGE 4 FIXES 1. Find the code location where this flaw of directly retrieving the profile
|
public void handleRequest( WebSession s )
|
||||||
* without data-level access control checking exists: public void handleRequest( WebSession s ) { …
|
{ …
|
||||||
* Employee employee = getEmployeeProfile(s, userId, employeeId); … } public Employee
|
Employee employee = getEmployeeProfile(s, userId, employeeId);
|
||||||
* getEmployeeProfile(WebSession s, int employeeId, int subjectUserId) throws UnauthorizedException
|
… }
|
||||||
* { … return getEmployeeProfile(s, employeeId, subjectUserId); … } 2. The solution requires a
|
public Employee getEmployeeProfile(WebSession s, int employeeId, int subjectUserId) throws UnauthorizedException { …
|
||||||
* data-level access control check to ensure the user has the rights to access the data they are
|
return getEmployeeProfile(s, employeeId, subjectUserId);
|
||||||
* requesting. a. There is a common method you can take advantage of: isAuthorizedForEmployee(s,
|
… }
|
||||||
* userId, subjectUserId) Either tell the student this exists or have them look in
|
2. The solution requires a data-level access control check to ensure the user has the rights to access the data they are requesting.
|
||||||
* DefaultLessonAction. Note that this is not required to implement data access control but is for
|
a. There is a common method you can take advantage of:
|
||||||
* detection of violations. b. Uncomment the modified query retrieving the user data to have data
|
isAuthorizedForEmployee(s, userId, subjectUserId)
|
||||||
* access control String query =
|
Either tell the student this exists or have them look in DefaultLessonAction.
|
||||||
* "SELECT * FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
Note that this is not required to implement data access control but is for detection of violations.
|
||||||
* "ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId; 3. Bundle
|
b. Uncomment the modified query retrieving the user data to have data access control
|
||||||
* the entire logic with this call and throw an unauthorized exception if
|
String query = "SELECT * FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
||||||
* (isAuthorizedForEmployee(s, userId, subjectUserId)) { ... //String query =
|
"ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId;
|
||||||
* "SELECT * FROM employee WHERE userid = " + subjectUserId; String query =
|
3. Bundle the entire logic with this call and throw an unauthorized exception
|
||||||
* "SELECT * FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
if (isAuthorizedForEmployee(s, userId, subjectUserId))
|
||||||
* "ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId; // STAGE 4
|
{ ...
|
||||||
* - FIX ... } else { throw new UnauthorizedException(); } 4. Repeat stage 3 and note that the
|
//String query = "SELECT * FROM employee WHERE userid = " + subjectUserId;
|
||||||
* function fails with a "Not authorized" message. Adding the access check in the query is providing
|
String query = "SELECT * FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
||||||
* data-level access control. The access check from isAuthorizedForEmployee is used to detect a
|
"ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId; // STAGE 4 - FIX
|
||||||
* violation. The same logic could've been applied after the query but isAuthorizedForEmployee
|
...
|
||||||
* provides a nice centralized abstraction of that logic.
|
}
|
||||||
*/
|
else
|
||||||
|
{
|
||||||
|
throw new UnauthorizedException();
|
||||||
|
}
|
||||||
|
4. Repeat stage 3 and note that the function fails with a "Not authorized" message.
|
||||||
|
Adding the access check in the query is providing data-level access control.
|
||||||
|
The access check from isAuthorizedForEmployee is used to detect a violation.
|
||||||
|
The same logic could've been applied after the query but isAuthorizedForEmployee provides a nice centralized abstraction of that logic.
|
||||||
|
*/
|
||||||
|
|
||||||
public class ViewProfile_i extends ViewProfile
|
public class ViewProfile_i extends ViewProfile
|
||||||
{
|
{
|
||||||
@ -58,8 +66,10 @@ public class ViewProfile_i extends ViewProfile
|
|||||||
// Query the database for the profile data of the given employee
|
// Query the database for the profile data of the given employee
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
// String query = "SELECT * FROM employee WHERE userid = " + subjectUserId; // STAGE
|
|
||||||
// 4 - FIX
|
// STAGE 4 - FIX
|
||||||
|
// String query = "SELECT * FROM employee WHERE userid = " + subjectUserId;
|
||||||
|
|
||||||
|
|
||||||
// Switch to this query to add Data Access Control
|
// Switch to this query to add Data Access Control
|
||||||
//
|
//
|
||||||
@ -67,11 +77,9 @@ public class ViewProfile_i extends ViewProfile
|
|||||||
// - qualify on ownership.employer_id to see only the current userId records
|
// - qualify on ownership.employer_id to see only the current userId records
|
||||||
// - qualify on ownership.employee_id to see the current selected employee profile
|
// - qualify on ownership.employee_id to see the current selected employee profile
|
||||||
|
|
||||||
|
// STAGE 4 - FIX
|
||||||
String query = "SELECT * FROM employee,ownership WHERE employee.userid = ownership.employee_id and "
|
String query = "SELECT * FROM employee,ownership WHERE employee.userid = ownership.employee_id and "
|
||||||
+ "ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId; // STAGE
|
+ "ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId;
|
||||||
// 4
|
|
||||||
// -
|
|
||||||
// FIX
|
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
@ -81,21 +89,29 @@ public class ViewProfile_i extends ViewProfile
|
|||||||
if (answer_results.next())
|
if (answer_results.next())
|
||||||
{
|
{
|
||||||
// Note: Do NOT get the password field.
|
// Note: Do NOT get the password field.
|
||||||
profile = new Employee(answer_results.getInt("userid"), answer_results.getString("first_name"),
|
profile = new Employee(
|
||||||
answer_results.getString("last_name"), answer_results.getString("ssn"), answer_results
|
answer_results.getInt("userid"),
|
||||||
.getString("title"), answer_results.getString("phone"), answer_results
|
answer_results.getString("first_name"),
|
||||||
.getString("address1"), answer_results.getString("address2"), answer_results
|
answer_results.getString("last_name"),
|
||||||
.getInt("manager"), answer_results.getString("start_date"), answer_results
|
answer_results.getString("ssn"),
|
||||||
.getInt("salary"), answer_results.getString("ccn"), answer_results
|
answer_results.getString("title"),
|
||||||
.getInt("ccn_limit"), answer_results.getString("disciplined_date"),
|
answer_results.getString("phone"),
|
||||||
answer_results.getString("disciplined_notes"), answer_results
|
answer_results.getString("address1"),
|
||||||
.getString("personal_description"));
|
answer_results.getString("address2"),
|
||||||
/*
|
answer_results.getInt("manager"),
|
||||||
* System.out.println("Retrieved employee from db: " +
|
answer_results.getString("start_date"),
|
||||||
* profile.getFirstName() + " " + profile.getLastName() + " (" +
|
answer_results.getInt("salary"),
|
||||||
* profile.getId() + ")");
|
answer_results.getString("ccn"),
|
||||||
*/}
|
answer_results.getInt("ccn_limit"),
|
||||||
} catch (SQLException sqle)
|
answer_results.getString("disciplined_date"),
|
||||||
|
answer_results.getString("disciplined_notes"),
|
||||||
|
answer_results.getString("personal_description"));
|
||||||
|
/* System.out.println("Retrieved employee from db: " +
|
||||||
|
profile.getFirstName() + " " + profile.getLastName() +
|
||||||
|
" (" + profile.getId() + ")");
|
||||||
|
*/ }
|
||||||
|
}
|
||||||
|
catch ( SQLException sqle )
|
||||||
{
|
{
|
||||||
s.setMessage("Error getting employee profile");
|
s.setMessage("Error getting employee profile");
|
||||||
sqle.printStackTrace();
|
sqle.printStackTrace();
|
||||||
|
@ -10,19 +10,25 @@ import org.owasp.webgoat.lessons.SQLInjection.Login;
|
|||||||
import org.owasp.webgoat.lessons.SQLInjection.SQLInjection;
|
import org.owasp.webgoat.lessons.SQLInjection.SQLInjection;
|
||||||
import org.owasp.webgoat.session.WebSession;
|
import org.owasp.webgoat.session.WebSession;
|
||||||
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Solution Summary: Edit Login.java and change login(). Modify login() with lines denoted by //
|
Solution Summary: Edit Login.java and change login().
|
||||||
* STAGE 2 - FIX. Solution Steps: 1. Change dynamic query to parameterized query. a. Replace the
|
Modify login() with lines denoted by // STAGE 2 - FIX.
|
||||||
* dynamic varaibles with the "?" String query =
|
Solution Steps:
|
||||||
* "SELECT * FROM employee WHERE userid = ? and password = ?" b. Create a preparedStatement using
|
1. Change dynamic query to parameterized query.
|
||||||
* the new query PreparedStatement answer_statement =
|
a. Replace the dynamic varaibles with the "?"
|
||||||
* SQLInjection.getConnection(s).prepareStatement( query, ResultSet.TYPE_SCROLL_INSENSITIVE,
|
String query = "SELECT * FROM employee WHERE userid = ? and password = ?"
|
||||||
* ResultSet.CONCUR_READ_ONLY ); c. Set the values of the parameterized query
|
|
||||||
* answer_statement.setString(1, userId); // STAGE 2 - FIX answer_statement.setString(2, password);
|
b. Create a preparedStatement using the new query
|
||||||
* // STAGE 2 - FIX d. Execute the preparedStatement ResultSet answer_results =
|
PreparedStatement answer_statement = SQLInjection.getConnection(s).prepareStatement(
|
||||||
* answer_statement.executeQuery();
|
query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY );
|
||||||
*/
|
|
||||||
|
c. Set the values of the parameterized query
|
||||||
|
answer_statement.setString(1, userId); // STAGE 2 - FIX
|
||||||
|
answer_statement.setString(2, password); // STAGE 2 - FIX
|
||||||
|
|
||||||
|
d. Execute the preparedStatement
|
||||||
|
ResultSet answer_results = answer_statement.executeQuery();
|
||||||
|
*/
|
||||||
|
|
||||||
public class Login_i extends Login
|
public class Login_i extends Login
|
||||||
{
|
{
|
||||||
@ -38,16 +44,15 @@ public class Login_i extends Login
|
|||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
String query = "SELECT * FROM employee WHERE userid = ? and password = ?"; // STAGE 2 -
|
// STAGE 2 - FIX
|
||||||
// FIX
|
String query = "SELECT * FROM employee WHERE userid = ? and password = ?";
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
|
|
||||||
|
// STAGE 2 - FIX
|
||||||
PreparedStatement answer_statement = WebSession.getConnection(s)
|
PreparedStatement answer_statement = WebSession.getConnection(s)
|
||||||
.prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); // STAGE
|
.prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
|
||||||
// 2
|
|
||||||
// -
|
|
||||||
// FIX
|
|
||||||
answer_statement.setString(1, userId); // STAGE 2 - FIX
|
answer_statement.setString(1, userId); // STAGE 2 - FIX
|
||||||
answer_statement.setString(2, password); // STAGE 2 - FIX
|
answer_statement.setString(2, password); // STAGE 2 - FIX
|
||||||
ResultSet answer_results = answer_statement.executeQuery(); // STAGE 2 - FIX
|
ResultSet answer_results = answer_statement.executeQuery(); // STAGE 2 - FIX
|
||||||
|
@ -11,24 +11,32 @@ import org.owasp.webgoat.session.UnauthorizedException;
|
|||||||
import org.owasp.webgoat.session.WebSession;
|
import org.owasp.webgoat.session.WebSession;
|
||||||
import org.owasp.webgoat.util.HtmlEncoder;
|
import org.owasp.webgoat.util.HtmlEncoder;
|
||||||
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Solution Summary: Edit ViewProfile.java and change getEmployeeProfile(). Modify
|
Solution Summary: Edit ViewProfile.java and change getEmployeeProfile().
|
||||||
* getEmployeeProfile() with lines denoted by // STAGE 4 - FIX. Solution Steps: 1. Change dynamic
|
Modify getEmployeeProfile() with lines denoted by // STAGE 4 - FIX.
|
||||||
* query to parameterized query. a. Replace the dynamic variables with the "?" Old: String query =
|
|
||||||
* "SELECT employee.* " +
|
Solution Steps:
|
||||||
* "FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
1. Change dynamic query to parameterized query.
|
||||||
* "ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId; New:
|
a. Replace the dynamic variables with the "?"
|
||||||
* String query = "SELECT employee.* " +
|
Old: String query = "SELECT employee.* " +
|
||||||
* "FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
"FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
||||||
* "ownership.employer_id = ? and ownership.employee_id = ?"; b. Create a preparedStatement using
|
"ownership.employer_id = " + userId + " and ownership.employee_id = " + subjectUserId;
|
||||||
* the new query PreparedStatement answer_statement =
|
|
||||||
* SQLInjection.getConnection(s).prepareStatement( query, ResultSet.TYPE_SCROLL_INSENSITIVE,
|
New: String query = "SELECT employee.* " +
|
||||||
* ResultSet.CONCUR_READ_ONLY ); c. Set the values of the parameterized query
|
"FROM employee,ownership WHERE employee.userid = ownership.employee_id and " +
|
||||||
* answer_statement.setInt(1, Integer.parseInt(userId)); // STAGE 4 - FIX answer_statement.setInt(2,
|
"ownership.employer_id = ? and ownership.employee_id = ?";
|
||||||
* Integer.parseInt(subjectUserId)); // STAGE 4 - FIX d. Execute the preparedStatement ResultSet
|
|
||||||
* answer_results = answer_statement.executeQuery();
|
b. Create a preparedStatement using the new query
|
||||||
*/
|
PreparedStatement answer_statement = SQLInjection.getConnection(s).prepareStatement(
|
||||||
|
query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY );
|
||||||
|
|
||||||
|
c. Set the values of the parameterized query
|
||||||
|
answer_statement.setInt(1, Integer.parseInt(userId)); // STAGE 4 - FIX
|
||||||
|
answer_statement.setInt(2, Integer.parseInt(subjectUserId)); // STAGE 4 - FIX
|
||||||
|
|
||||||
|
d. Execute the preparedStatement
|
||||||
|
ResultSet answer_results = answer_statement.executeQuery();
|
||||||
|
*/
|
||||||
|
|
||||||
public class ViewProfile_i extends ViewProfile
|
public class ViewProfile_i extends ViewProfile
|
||||||
{
|
{
|
||||||
@ -53,30 +61,38 @@ public class ViewProfile_i extends ViewProfile
|
|||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
PreparedStatement answer_statement = WebSession.getConnection(s)
|
// STAGE 4 - FIX
|
||||||
.prepareStatement(query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); // STAGE
|
PreparedStatement answer_statement = WebSession.getConnection(s).prepareStatement( query,
|
||||||
// 4
|
ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY );
|
||||||
// -
|
|
||||||
// FIX
|
|
||||||
answer_statement.setInt(1, Integer.parseInt(userId)); // STAGE 4 - FIX
|
answer_statement.setInt(1, Integer.parseInt(userId)); // STAGE 4 - FIX
|
||||||
answer_statement.setInt(2, Integer.parseInt(subjectUserId)); // STAGE 4 - FIX
|
answer_statement.setInt(2, Integer.parseInt(subjectUserId)); // STAGE 4 - FIX
|
||||||
ResultSet answer_results = answer_statement.executeQuery(); // STAGE 4 - FIX
|
ResultSet answer_results = answer_statement.executeQuery(); // STAGE 4 - FIX
|
||||||
if (answer_results.next())
|
if (answer_results.next())
|
||||||
{
|
{
|
||||||
// Note: Do NOT get the password field.
|
// Note: Do NOT get the password field.
|
||||||
profile = new Employee(answer_results.getInt("userid"), answer_results.getString("first_name"),
|
profile = new Employee(
|
||||||
answer_results.getString("last_name"), answer_results.getString("ssn"), answer_results
|
answer_results.getInt("userid"),
|
||||||
.getString("title"), answer_results.getString("phone"), answer_results
|
answer_results.getString("first_name"),
|
||||||
.getString("address1"), answer_results.getString("address2"), answer_results
|
answer_results.getString("last_name"),
|
||||||
.getInt("manager"), answer_results.getString("start_date"), answer_results
|
answer_results.getString("ssn"),
|
||||||
.getInt("salary"), answer_results.getString("ccn"), answer_results
|
answer_results.getString("title"),
|
||||||
.getInt("ccn_limit"), answer_results.getString("disciplined_date"), answer_results
|
answer_results.getString("phone"),
|
||||||
.getString("disciplined_notes"), answer_results.getString("personal_description"));
|
answer_results.getString("address1"),
|
||||||
/*
|
answer_results.getString("address2"),
|
||||||
* System.out.println("Retrieved employee from db: " + profile.getFirstName() +
|
answer_results.getInt("manager"),
|
||||||
* " " + profile.getLastName() + " (" + profile.getId() + ")");
|
answer_results.getString("start_date"),
|
||||||
*/}
|
answer_results.getInt("salary"),
|
||||||
} catch (SQLException sqle)
|
answer_results.getString("ccn"),
|
||||||
|
answer_results.getInt("ccn_limit"),
|
||||||
|
answer_results.getString("disciplined_date"),
|
||||||
|
answer_results.getString("disciplined_notes"),
|
||||||
|
answer_results.getString("personal_description"));
|
||||||
|
/* System.out.println("Retrieved employee from db: " +
|
||||||
|
profile.getFirstName() + " " + profile.getLastName() +
|
||||||
|
" (" + profile.getId() + ")");
|
||||||
|
*/ }
|
||||||
|
}
|
||||||
|
catch ( SQLException sqle )
|
||||||
{
|
{
|
||||||
s.setMessage("Error getting employee profile");
|
s.setMessage("Error getting employee profile");
|
||||||
sqle.printStackTrace();
|
sqle.printStackTrace();
|
||||||
|
@ -129,7 +129,8 @@ public class Course
|
|||||||
fileName = fileName.trim();
|
fileName = fileName.trim();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* We do not handle directories. We do not handle files with different extensions
|
* We do not handle directories.
|
||||||
|
* We do not handle files with different extensions
|
||||||
*/
|
*/
|
||||||
if (fileName.endsWith("/") || !fileName.endsWith(ext)) { return null; }
|
if (fileName.endsWith("/") || !fileName.endsWith(ext)) { return null; }
|
||||||
|
|
||||||
|
@ -184,8 +184,7 @@ public class UserTracker
|
|||||||
usermap.put(screen.getTitle(), tracker);
|
usermap.put(screen.getTitle(), tracker);
|
||||||
}
|
}
|
||||||
// System.out.println( "User: [" + userName + "] UserTracker:getLessonTracker() LTH " +
|
// System.out.println( "User: [" + userName + "] UserTracker:getLessonTracker() LTH " +
|
||||||
// tracker.hashCode() + "
|
// tracker.hashCode() + " for " + screen );
|
||||||
// for " + screen );
|
|
||||||
return tracker;
|
return tracker;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -261,9 +260,8 @@ public class UserTracker
|
|||||||
|
|
||||||
LessonTracker tracker = getLessonTracker(s, screen);
|
LessonTracker tracker = getLessonTracker(s, screen);
|
||||||
|
|
||||||
// System.out.println( "User [" + s.getUserName() + "] TRACKER: updating " + screen + " LTH
|
// System.out.println( "User [" + s.getUserName() + "] TRACKER: updating " + screen +
|
||||||
// " +
|
// " LTH " + tracker.hashCode() );
|
||||||
// tracker.hashCode() );
|
|
||||||
tracker.store(s, screen);
|
tracker.store(s, screen);
|
||||||
|
|
||||||
HashMap<String, LessonTracker> usermap = getUserMap(s.getUserName());
|
HashMap<String, LessonTracker> usermap = getUserMap(s.getUserName());
|
||||||
|
@ -8,7 +8,7 @@ SQL injection attacks represent a serious threat to any database-driven site. Th
|
|||||||
<br><br>
|
<br><br>
|
||||||
Not only is it a threat easily instigated, it is also a threat that, with a little common-sense and forethought, can easily be prevented.<br>
|
Not only is it a threat easily instigated, it is also a threat that, with a little common-sense and forethought, can easily be prevented.<br>
|
||||||
<br>
|
<br>
|
||||||
It is always good practice to sanitize all input data, especially data that will used in OS command, scripts, and database queiries, even if the threat of SQL injection has been prevented in some other manner.<br>
|
It is always good practice to sanitize all input data, especially data that will used in OS command, scripts, and database queries, even if the threat of SQL injection has been prevented in some other manner.<br>
|
||||||
<p><b>General Goal(s):</b> </p>
|
<p><b>General Goal(s):</b> </p>
|
||||||
The form below allows a user to view weather data. Try to inject an SQL string that results in all the weather data being displayed.
|
The form below allows a user to view weather data. Try to inject an SQL string that results in all the weather data being displayed.
|
||||||
<!-- Stop Instructions -->
|
<!-- Stop Instructions -->
|
@ -8,7 +8,7 @@ SQL injection attacks represent a serious threat to any database-driven site. Th
|
|||||||
<br><br>
|
<br><br>
|
||||||
Not only is it a threat easily instigated, it is also a threat that, with a little common-sense and forethought, can easily be prevented.<br>
|
Not only is it a threat easily instigated, it is also a threat that, with a little common-sense and forethought, can easily be prevented.<br>
|
||||||
<br>
|
<br>
|
||||||
It is always good practice to sanitize all input data, especially data that will used in OS command, scripts, and database queiries, even if the threat of SQL injection has been prevented in some other manner.<br>
|
It is always good practice to sanitize all input data, especially data that will used in OS command, scripts, and database queries, even if the threat of SQL injection has been prevented in some other manner.<br>
|
||||||
<p><b>General Goal(s):</b> </p>
|
<p><b>General Goal(s):</b> </p>
|
||||||
The form below allows a user to view their credit card numbers. Try to inject an SQL string that results in all the credit card numbers being displayed. Try the user name of 'Smith'.
|
The form below allows a user to view their credit card numbers. Try to inject an SQL string that results in all the credit card numbers being displayed. Try the user name of 'Smith'.
|
||||||
<!-- Stop Instructions -->
|
<!-- Stop Instructions -->
|
@ -599,7 +599,7 @@ SQL injection attacks represent a serious threat to any database-driven site. Th
|
|||||||
|
|
||||||
Not only is it a threat easily instigated, it is also a threat that, with a little common-sense and forethought, can easily be prevented.
|
Not only is it a threat easily instigated, it is also a threat that, with a little common-sense and forethought, can easily be prevented.
|
||||||
|
|
||||||
It is always good practice to sanitize all input data, especially data that will used in OS command, scripts, and database queiries, even if the threat of SQL injection has been prevented in some other manner.<o:p></o:p></span></p>
|
It is always good practice to sanitize all input data, especially data that will used in OS command, scripts, and database queries, even if the threat of SQL injection has been prevented in some other manner.<o:p></o:p></span></p>
|
||||||
<p class=MsoNormal><span style='font-family:"Arial","sans-serif"'><o:p> </o:p></span></p>
|
<p class=MsoNormal><span style='font-family:"Arial","sans-serif"'><o:p> </o:p></span></p>
|
||||||
<p class=MsoNormal><b><span style='font-family:"Arial","sans-serif"'>General
|
<p class=MsoNormal><b><span style='font-family:"Arial","sans-serif"'>General
|
||||||
Goal(s):</span></b><span style='font-family:"Arial","sans-serif"'> <o:p></o:p></span></p>
|
Goal(s):</span></b><span style='font-family:"Arial","sans-serif"'> <o:p></o:p></span></p>
|
||||||
|
@ -602,7 +602,7 @@ Not only is it a threat easily instigated, it is also a threat that, with a
|
|||||||
little common-sense and forethought, can easily be prevented.<br>
|
little common-sense and forethought, can easily be prevented.<br>
|
||||||
<br>
|
<br>
|
||||||
It is always good practice to sanitize all input data, especially data that
|
It is always good practice to sanitize all input data, especially data that
|
||||||
will used in OS command, scripts, and database queiries, even if the threat of
|
will used in OS command, scripts, and database queries, even if the threat of
|
||||||
SQL injection has been prevented in some other manner.<o:p></o:p></span></p>
|
SQL injection has been prevented in some other manner.<o:p></o:p></span></p>
|
||||||
|
|
||||||
<p class=MsoNormal><b><span style='font-family:"Arial","sans-serif"'><o:p> </o:p></span></b></p>
|
<p class=MsoNormal><b><span style='font-family:"Arial","sans-serif"'><o:p> </o:p></span></b></p>
|
||||||
|
Loading…
x
Reference in New Issue
Block a user