From ac4b06f11b7a0a3e8ed25b13e5682e4adc26d2f8 Mon Sep 17 00:00:00 2001 From: Nanne Baars Date: Sun, 19 Dec 2021 14:35:14 +0100 Subject: [PATCH] Move enabling security to WebGoat core and add resetting the lessons. We can use it for more lessons and showcase how to apply security directly from the source code. Resolves: #1176 --- .../owasp/webgoat/service/SessionService.java | 61 +++++-------------- .../org/owasp/webgoat/session/WebSession.java | 17 ++++-- .../main/resources/i18n/messages.properties | 4 +- .../test/java/org/owasp/webgoat/XXETest.java | 21 +++++-- .../webgoat/xxe/BlindSendFileAssignment.java | 6 +- .../java/org/owasp/webgoat/xxe/Comments.java | 4 +- .../webgoat/xxe/ContentTypeAssignment.java | 6 +- .../java/org/owasp/webgoat/xxe/SimpleXXE.java | 31 +++------- .../main/java/org/owasp/webgoat/xxe/User.java | 9 --- .../xxe/src/main/resources/html/XXE.html | 4 +- 10 files changed, 60 insertions(+), 103 deletions(-) diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/service/SessionService.java b/webgoat-container/src/main/java/org/owasp/webgoat/service/SessionService.java index 5d0f4b3cc..e621270ce 100644 --- a/webgoat-container/src/main/java/org/owasp/webgoat/service/SessionService.java +++ b/webgoat-container/src/main/java/org/owasp/webgoat/service/SessionService.java @@ -6,57 +6,28 @@ package org.owasp.webgoat.service; +import lombok.RequiredArgsConstructor; +import org.owasp.webgoat.i18n.Messages; +import org.owasp.webgoat.session.WebSession; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSession; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Date; -import java.util.Enumeration; -import java.util.List; - -/** - *

SessionService class.

- * - * @author rlawson - * @version $Id: $Id - */ @Controller +@RequiredArgsConstructor public class SessionService { - /** - * Returns hints for current lesson - * - * @param session a {@link javax.servlet.http.HttpSession} object. - * @param request a {@link javax.servlet.http.HttpServletRequest} object. - * @return a {@link java.lang.String} object. - */ - @RequestMapping(path = "/service/session.mvc", produces = "application/json") - public @ResponseBody - String showSession(HttpServletRequest request, HttpSession session) { - StringBuilder sb = new StringBuilder(); - sb.append("id").append(" = ").append(session.getId()).append("\n"); - sb.append("created").append(" = ").append(new Date(session.getCreationTime())).append("\n"); - sb.append("last access").append(" = ").append(new Date(session.getLastAccessedTime())).append("\n"); - sb.append("timeout (secs)").append(" = ").append(session.getMaxInactiveInterval()).append("\n"); - sb.append("session from cookie?").append(" = ").append(request.isRequestedSessionIdFromCookie()).append("\n"); - sb.append("session from url?").append(" = ").append(request.isRequestedSessionIdFromURL()).append("\n"); - sb.append("=====================================\n"); - // get attributes - List attributes = new ArrayList(); - Enumeration keys = session.getAttributeNames(); - while (keys.hasMoreElements()) { - String name = (String) keys.nextElement(); - attributes.add(name); - } - Collections.sort(attributes); - for (String attribute : attributes) { - String value = session.getAttribute(attribute) + ""; - sb.append(attribute).append(" = ").append(value).append("\n"); - } - return sb.toString(); + private final WebSession webSession; + private final RestartLessonService restartLessonService; + private final Messages messages; + + @RequestMapping(path = "/service/enable-security.mvc", produces = "application/json") + @ResponseBody + public String applySecurity() { + webSession.toggleSecurity(); + restartLessonService.restartLesson(); + + var msg = webSession.isSecurityEnabled() ? "security.enabled" : "security.disabled"; + return messages.getMessage(msg); } } diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/session/WebSession.java b/webgoat-container/src/main/java/org/owasp/webgoat/session/WebSession.java index 2cdba739c..d7a64984b 100644 --- a/webgoat-container/src/main/java/org/owasp/webgoat/session/WebSession.java +++ b/webgoat-container/src/main/java/org/owasp/webgoat/session/WebSession.java @@ -5,8 +5,6 @@ import org.owasp.webgoat.users.WebGoatUser; import org.springframework.security.core.context.SecurityContextHolder; import java.io.Serializable; -import java.sql.Connection; -import java.sql.SQLException; /** * ************************************************************************************************* @@ -39,9 +37,10 @@ import java.sql.SQLException; */ public class WebSession implements Serializable { - private static final long serialVersionUID = -4270066103101711560L; - private final WebGoatUser currentUser; - private Lesson currentLesson; + private static final long serialVersionUID = -4270066103101711560L; + private final WebGoatUser currentUser; + private transient Lesson currentLesson; + private boolean securityEnabled; public WebSession() { this.currentUser = (WebGoatUser) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); @@ -73,4 +72,12 @@ public class WebSession implements Serializable { public String getUserName() { return currentUser.getUsername(); } + + public void toggleSecurity() { + this.securityEnabled = !this.securityEnabled; + } + + public boolean isSecurityEnabled() { + return securityEnabled; + } } diff --git a/webgoat-container/src/main/resources/i18n/messages.properties b/webgoat-container/src/main/resources/i18n/messages.properties index d58990c3e..0c76dea40 100644 --- a/webgoat-container/src/main/resources/i18n/messages.properties +++ b/webgoat-container/src/main/resources/i18n/messages.properties @@ -60,4 +60,6 @@ not.empty=This field is required. username.size=Please use between 6 and 10 characters. username.duplicate=User already exists. password.size=Password should at least contain 6 characters -password.diff=The passwords do not match. \ No newline at end of file +password.diff=The passwords do not match. +security.enabled=Security enabled, you can try the previous challenges and see the effect! +security.disabled=Security enabled, you can try the previous challenges and see the effect! diff --git a/webgoat-integration-tests/src/test/java/org/owasp/webgoat/XXETest.java b/webgoat-integration-tests/src/test/java/org/owasp/webgoat/XXETest.java index f06481591..5e5d3a0fb 100644 --- a/webgoat-integration-tests/src/test/java/org/owasp/webgoat/XXETest.java +++ b/webgoat-integration-tests/src/test/java/org/owasp/webgoat/XXETest.java @@ -12,10 +12,14 @@ import io.restassured.http.ContentType; public class XXETest extends IntegrationTest { - private static final String xxe3 = "]>&xxe;test"; - private static final String xxe4 = "]>&xxe;test"; - private static final String dtd7 = "\">%all;"; - private static final String xxe7 = "%remote;]>test&send;"; + private static final String xxe3 = """ + ]>&xxe;test"""; + private static final String xxe4 = """ + ]>&xxe;test"""; + private static final String dtd7 = """ + ">%all;"""; + private static final String xxe7 = """ + %remote;]>test&send;"""; private String webGoatHomeDirectory; private String webwolfFileDir; @@ -39,8 +43,13 @@ public class XXETest extends IntegrationTest { startLesson("XXE"); webGoatHomeDirectory = getWebGoatServerPath(); webwolfFileDir = getWebWolfServerPath(); - RestAssured.given().when().relaxedHTTPSValidation() - .cookie("JSESSIONID", getWebGoatCookie()).get(url("/xxe/applysecurity")); + RestAssured.given() + .when() + .relaxedHTTPSValidation() + .cookie("JSESSIONID", getWebGoatCookie()) + .get(url("service/enable-security.mvc")) + .then() + .statusCode(200); checkAssignment(url("/WebGoat/xxe/simple"), ContentType.XML, xxe3, false); checkAssignment(url("/WebGoat/xxe/content-type"), ContentType.XML, xxe4, false); checkAssignment(url("/WebGoat/xxe/blind"), ContentType.XML, "" + getSecret() + "", false); diff --git a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/BlindSendFileAssignment.java b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/BlindSendFileAssignment.java index f18bb83ba..17f3b813a 100644 --- a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/BlindSendFileAssignment.java +++ b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/BlindSendFileAssignment.java @@ -83,11 +83,7 @@ public class BlindSendFileAssignment extends AssignmentEndpoint { } try { - boolean secure = false; - if (null != request.getSession().getAttribute("applySecurity")) { - secure = true; - } - Comment comment = comments.parseXml(commentStr, secure); + Comment comment = comments.parseXml(commentStr); if (CONTENTS.contains(comment.getText())) { comment.setText("Nice try, you need to send the file to WebWolf"); } diff --git a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/Comments.java b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/Comments.java index a86cbd6be..fb985b4c9 100644 --- a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/Comments.java +++ b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/Comments.java @@ -89,11 +89,11 @@ public class Comments { * XmlMapper bean defined above will be used automatically and the Comment class can be directly used in the * controller method (instead of a String) */ - protected Comment parseXml(String xml, boolean secure) throws JAXBException, XMLStreamException { + protected Comment parseXml(String xml) throws JAXBException, XMLStreamException { var jc = JAXBContext.newInstance(Comment.class); var xif = XMLInputFactory.newInstance(); - if (secure) { + if (webSession.isSecurityEnabled()) { xif.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant xif.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant } diff --git a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/ContentTypeAssignment.java b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/ContentTypeAssignment.java index 4283c2895..052dbe772 100644 --- a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/ContentTypeAssignment.java +++ b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/ContentTypeAssignment.java @@ -68,11 +68,7 @@ public class ContentTypeAssignment extends AssignmentEndpoint { if (null != contentType && contentType.contains(MediaType.APPLICATION_XML_VALUE)) { String error = ""; try { - boolean secure = false; - if (null != request.getSession().getAttribute("applySecurity")) { - secure = true; - } - Comment comment = comments.parseXml(commentStr, secure); + Comment comment = comments.parseXml(commentStr); comments.addComment(comment, false); if (checkSolution(comment)) { attackResult = success(this).build(); diff --git a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/SimpleXXE.java b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/SimpleXXE.java index c46b0504a..f77eb5d07 100644 --- a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/SimpleXXE.java +++ b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/SimpleXXE.java @@ -30,7 +30,6 @@ import org.owasp.webgoat.assignments.AttackResult; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.MediaType; -import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -66,15 +65,10 @@ public class SimpleXXE extends AssignmentEndpoint { @PostMapping(path = "xxe/simple", consumes = ALL_VALUE, produces = APPLICATION_JSON_VALUE) @ResponseBody - public AttackResult createNewComment(HttpServletRequest request, @RequestBody String commentStr) throws Exception { + public AttackResult createNewComment(HttpServletRequest request, @RequestBody String commentStr) { String error = ""; try { - boolean secure = false; - if (null != request.getSession().getAttribute("applySecurity")) { - secure = true; - } - Comment comment = comments.parseXml(commentStr, secure); - //System.err.println("Comment " + comment); + var comment = comments.parseXml(commentStr); comments.addComment(comment, false); if (checkSolution(comment)) { return success(this).build(); @@ -103,21 +97,12 @@ public class SimpleXXE extends AssignmentEndpoint { @RequestMapping(path = "/xxe/sampledtd", consumes = ALL_VALUE, produces = MediaType.TEXT_PLAIN_VALUE) @ResponseBody public String getSampleDTDFile() { - return "\n" - + "\n" - + "\">\n" - + "%all;"; - } - - @GetMapping(path="/xxe/applysecurity",produces=MediaType.TEXT_PLAIN_VALUE) - @ResponseBody - public String setSecurity(HttpServletRequest request) { - - String applySecurity = (String) request.getSession().getAttribute("applySecurity"); - if (applySecurity == null) { - request.getSession().setAttribute("applySecurity", "true"); - } - return "xxe security patch is now applied, you can try the previous challenges and see the effect!"; + return """ + + + "> + %all; + """; } } diff --git a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/User.java b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/User.java index fd10e7fea..32e55b050 100644 --- a/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/User.java +++ b/webgoat-lessons/xxe/src/main/java/org/owasp/webgoat/xxe/User.java @@ -29,7 +29,6 @@ public class User { private String username = ""; private String password = ""; - private String email = ""; public String getPassword() { return password; @@ -47,12 +46,4 @@ public class User { this.username = username; } - public String getEmail() { - return email; - } - - public void setEmail(String email) { - this.email = email; - } - } diff --git a/webgoat-lessons/xxe/src/main/resources/html/XXE.html b/webgoat-lessons/xxe/src/main/resources/html/XXE.html index 20a232b39..e2cc3685e 100644 --- a/webgoat-lessons/xxe/src/main/resources/html/XXE.html +++ b/webgoat-lessons/xxe/src/main/resources/html/XXE.html @@ -221,8 +221,8 @@

- Apply XXE security patch + Apply XXE security patch
- \ No newline at end of file +