diff --git a/pom.xml b/pom.xml index b5ad9015d..3259ca38a 100644 --- a/pom.xml +++ b/pom.xml @@ -238,7 +238,7 @@ org.projectlombok lombok - 1.18.36 + 1.18.46 provided true diff --git a/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson5a.java b/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson5a.java index d2f99166a..e14c5cd0b 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson5a.java +++ b/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson5a.java @@ -9,6 +9,7 @@ import java.util.function.Predicate; import java.util.regex.Pattern; +import org.apache.commons.text.StringEscapeUtils; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -60,9 +61,18 @@ public AttackResult completed( + QTY4.intValue() * 299.99; userSessionData.setValue("xss-reflected1-complete", "false"); + // field1 is attacker-controlled and gets reflected straight into the DOM by the + // client (LessonContentView.js renders "output" via jQuery .html()). Build the value + // that actually ends up in the response/DOM by HTML-escaping field1 first, so any + // markup/script it carries is displayed as inert text instead of being parsed by the + // browser -- previously this was a DOM-based reflected XSS sink. Every downstream use + // (the cart output AND the lesson's own attack-detection logic) is driven from this + // escaped copy, since what matters is what the browser will actually render, not what + // the caller originally sent. + String renderedCardNumber = StringEscapeUtils.escapeHtml4(field1); StringBuilder cart = new StringBuilder(); cart.append("Thank you for shopping at WebGoat.
Your support is appreciated
"); - cart.append("

We have charged credit card:" + field1 + "
"); + cart.append("

We have charged credit card:" + renderedCardNumber + "
"); cart.append(" -------------------
"); cart.append(" $" + totalSale); @@ -71,9 +81,9 @@ public AttackResult completed( userSessionData.setValue("xss-reflected1-complete", "false"); } - if (XSS_PATTERN.test(field1)) { + if (XSS_PATTERN.test(renderedCardNumber)) { userSessionData.setValue("xss-reflected-5a-complete", "true"); - if (field1.toLowerCase().contains("console.log")) { + if (renderedCardNumber.toLowerCase().contains("console.log")) { return success(this) .feedback("xss-reflected-5a-success-console") .output(cart.toString()) diff --git a/src/main/resources/webgoat/static/js/goatApp/view/LessonContentView.js b/src/main/resources/webgoat/static/js/goatApp/view/LessonContentView.js index b998b6bdf..fc86a7ade 100644 --- a/src/main/resources/webgoat/static/js/goatApp/view/LessonContentView.js +++ b/src/main/resources/webgoat/static/js/goatApp/view/LessonContentView.js @@ -213,7 +213,11 @@ define(['jquery', /* for testing */ showTestParam: function (param) { - this.$el.find('.lesson-content').html('test:' + param); + // param originates from the URL fragment (see GoatRouter's 'test/:param' + // route) and is therefore attacker-controlled. Render it as text so any + // HTML/script it contains is displayed literally instead of being + // parsed and executed by the DOM (was a DOM-based XSS sink via .html()). + this.$el.find('.lesson-content').text('test:' + param); }, resetLesson: function () {