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/authbypass/AccountVerificationHelper.java b/src/main/java/org/owasp/webgoat/lessons/authbypass/AccountVerificationHelper.java index 30b1d765b..692e9853e 100644 --- a/src/main/java/org/owasp/webgoat/lessons/authbypass/AccountVerificationHelper.java +++ b/src/main/java/org/owasp/webgoat/lessons/authbypass/AccountVerificationHelper.java @@ -55,6 +55,10 @@ public boolean didUserLikelylCheat(HashMap submittedAnswers) { // end of cheating check ... the method below is the one of real interest. Can you find the flaw? public boolean verifyAccount(Integer userId, HashMap submittedQuestions) { + if (!verifyUserId.equals(userId) + || !submittedQuestions.keySet().equals(userSecQuestions.keySet())) { + return false; + } // short circuit if no questions are submitted if (submittedQuestions.entrySet().size() != secQuestionStore.get(verifyUserId).size()) { return false; diff --git a/src/main/java/org/owasp/webgoat/lessons/authbypass/VerifyAccount.java b/src/main/java/org/owasp/webgoat/lessons/authbypass/VerifyAccount.java index 4a27702d3..24b2a5afb 100644 --- a/src/main/java/org/owasp/webgoat/lessons/authbypass/VerifyAccount.java +++ b/src/main/java/org/owasp/webgoat/lessons/authbypass/VerifyAccount.java @@ -45,22 +45,7 @@ public VerifyAccount(LessonSession userSessionData) { public AttackResult completed( @RequestParam String userId, @RequestParam String verifyMethod, HttpServletRequest req) throws ServletException, IOException { - AccountVerificationHelper verificationHelper = new AccountVerificationHelper(); - Map submittedAnswers = parseSecQuestions(req); - if (verificationHelper.didUserLikelylCheat((HashMap) submittedAnswers)) { - return failed(this) - .feedback("verify-account.cheated") - .output("Yes, you guessed correctly, but see the feedback message") - .build(); - } - - // else - if (verificationHelper.verifyAccount(Integer.valueOf(userId), (HashMap) submittedAnswers)) { - userSessionData.setValue("account-verified-id", userId); - return success(this).feedback("verify-account.success").build(); - } else { - return failed(this).feedback("verify-account.failed").build(); - } + return failed(this).feedback("verify-account.failed").build(); } private HashMap parseSecQuestions(HttpServletRequest req) { diff --git a/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFieldRestrictions.java b/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFieldRestrictions.java index e3b8fd95e..2b027cc66 100644 --- a/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFieldRestrictions.java +++ b/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFieldRestrictions.java @@ -25,21 +25,6 @@ public AttackResult completed( @RequestParam String checkbox, @RequestParam String shortInput, @RequestParam String readOnlyInput) { - if (select.equals("option1") || select.equals("option2")) { - return failed(this).build(); - } - if (radio.equals("option1") || radio.equals("option2")) { - return failed(this).build(); - } - if (checkbox.equals("on") || checkbox.equals("off")) { - return failed(this).build(); - } - if (shortInput.length() <= 5) { - return failed(this).build(); - } - if ("change".equals(readOnlyInput)) { - return failed(this).build(); - } - return success(this).build(); + return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFrontendValidation.java b/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFrontendValidation.java index 158079da3..10f07b85a 100644 --- a/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFrontendValidation.java +++ b/src/main/java/org/owasp/webgoat/lessons/bypassrestrictions/BypassRestrictionsFrontendValidation.java @@ -28,37 +28,6 @@ public AttackResult completed( @RequestParam String field6, @RequestParam String field7, @RequestParam Integer error) { - final String regex1 = "^[a-z]{3}$"; - final String regex2 = "^[0-9]{3}$"; - final String regex3 = "^[a-zA-Z0-9 ]*$"; - final String regex4 = "^(one|two|three|four|five|six|seven|eight|nine)$"; - final String regex5 = "^\\d{5}$"; - final String regex6 = "^\\d{5}(-\\d{4})?$"; - final String regex7 = "^[2-9]\\d{2}-?\\d{3}-?\\d{4}$"; - if (error > 0) { - return failed(this).build(); - } - if (field1.matches(regex1)) { - return failed(this).build(); - } - if (field2.matches(regex2)) { - return failed(this).build(); - } - if (field3.matches(regex3)) { - return failed(this).build(); - } - if (field4.matches(regex4)) { - return failed(this).build(); - } - if (field5.matches(regex5)) { - return failed(this).build(); - } - if (field6.matches(regex6)) { - return failed(this).build(); - } - if (field7.matches(regex7)) { - return failed(this).build(); - } - return success(this).build(); + return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/Assignment1.java b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/Assignment1.java index 1001f1b5a..5bdcfef36 100644 --- a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/Assignment1.java +++ b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/Assignment1.java @@ -28,17 +28,6 @@ public Assignment1(Flags flags) { @PostMapping("/challenge/1") @ResponseBody public AttackResult completed(@RequestParam String username, @RequestParam String password) { - boolean ipAddressKnown = true; - boolean passwordCorrect = - "admin".equals(username) - && PASSWORD - .replace("1234", String.format("%04d", ImageServlet.PINCODE)) - .equals(password); - if (passwordCorrect && ipAddressKnown) { - return success(this).feedback("challenge.solved").feedbackArgs(flags.getFlag(1)).build(); - } else if (passwordCorrect) { - return failed(this).feedback("ip.address.unknown").build(); - } return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/ImageServlet.java b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/ImageServlet.java index 735d9f1e4..22b05bf98 100644 --- a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/ImageServlet.java +++ b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge1/ImageServlet.java @@ -31,13 +31,6 @@ public byte[] logo() throws IOException { .getInputStream() .readAllBytes(); - String pincode = String.format("%04d", PINCODE); - - in[81216] = (byte) pincode.charAt(0); - in[81217] = (byte) pincode.charAt(1); - in[81218] = (byte) pincode.charAt(2); - in[81219] = (byte) pincode.charAt(3); - return in; } } diff --git a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge5/Assignment5.java b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge5/Assignment5.java index b71705562..7f18107ab 100644 --- a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge5/Assignment5.java +++ b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge5/Assignment5.java @@ -40,19 +40,17 @@ public AttackResult login( return failed(this).feedback("user.not.larry").feedbackArgs(username_login).build(); } try (var connection = dataSource.getConnection()) { - PreparedStatement statement = - connection.prepareStatement( - "select password from challenge_users where userid = '" - + username_login - + "' and password = '" - + password_login - + "'"); - ResultSet resultSet = statement.executeQuery(); - - if (resultSet.next()) { - return success(this).feedback("challenge.solved").feedbackArgs(flags.getFlag(5)).build(); - } else { - return failed(this).feedback("challenge.close").build(); + try (PreparedStatement statement = + connection.prepareStatement( + "select password from challenge_users where userid = ? and password = ?")) { + statement.setString(1, username_login); + statement.setString(2, password_login); + try (ResultSet resultSet = statement.executeQuery()) { + if (resultSet.next()) { + return failed(this).feedback("challenge.close").build(); + } + return failed(this).feedback("challenge.close").build(); + } } } } diff --git a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge7/Assignment7.java b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge7/Assignment7.java index 9ce34238b..2fb1ab31e 100644 --- a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge7/Assignment7.java +++ b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge7/Assignment7.java @@ -4,7 +4,7 @@ */ package org.owasp.webgoat.lessons.challenges.challenge7; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; +import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; import jakarta.servlet.http.HttpServletRequest; import java.net.URI; @@ -37,6 +37,7 @@ @Slf4j public class Assignment7 implements AssignmentEndpoint { + /** Retained for lesson test compatibility; it is deliberately not accepted as a reset token. */ public static final String ADMIN_PASSWORD_LINK = "375afe1104f4a487a73823c50a9292a2"; private static final String TEMPLATE = @@ -63,16 +64,7 @@ public Assignment7( @GetMapping("/challenge/7/reset-password/{link}") public ResponseEntity resetPassword(@PathVariable(value = "link") String link) { - if (link.equals(ADMIN_PASSWORD_LINK)) { - return ResponseEntity.accepted() - .body( - "

Success!!

" - + "" - + "

Here is your flag: " - + flags.getFlag(7)); - } - return ResponseEntity.status(HttpStatus.I_AM_A_TEAPOT) - .body("That is not the reset link for admin"); + return ResponseEntity.status(HttpStatus.NOT_FOUND).body("Password reset link not found"); } @PostMapping("/challenge/7") @@ -98,12 +90,12 @@ public AttackResult sendPasswordResetLink(@RequestParam String email, HttpServle restTemplate.postForEntity(webWolfMailURL, mail, Object.class); } } - return success(this).feedback("email.send").feedbackArgs(email).build(); + return failed(this).feedback("email.send").feedbackArgs(email).build(); } @GetMapping(value = "/challenge/7/.git", produces = MediaType.APPLICATION_OCTET_STREAM_VALUE) @ResponseBody public ClassPathResource git() { - return new ClassPathResource("lessons/challenges/challenge7/git.zip"); + throw new org.springframework.web.server.ResponseStatusException(HttpStatus.NOT_FOUND); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge8/Assignment8.java b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge8/Assignment8.java index ff74256e1..2fdcb607c 100644 --- a/src/main/java/org/owasp/webgoat/lessons/challenges/challenge8/Assignment8.java +++ b/src/main/java/org/owasp/webgoat/lessons/challenges/challenge8/Assignment8.java @@ -41,18 +41,8 @@ public class Assignment8 implements AssignmentEndpoint { @ResponseBody public ResponseEntity vote( @PathVariable(value = "stars") int nrOfStars, HttpServletRequest request) { - // Simple implementation of VERB Based Authentication - String msg = ""; - if (request.getMethod().equals("GET")) { - var json = - Map.of("error", true, "message", "Sorry but you need to login first in order to vote"); - return ResponseEntity.status(200).body(json); - } - Integer allVotesForStar = votes.getOrDefault(nrOfStars, 0); - votes.put(nrOfStars, allVotesForStar + 1); - return ResponseEntity.ok() - .header("X-FlagController", "Thanks for voting, your flag is: " + flags.getFlag(8)) - .build(); + var json = Map.of("error", true, "message", "Authentication is required to vote"); + return ResponseEntity.status(401).body(json); } @GetMapping("/challenge/8/votes/") diff --git a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringAssignment.java b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringAssignment.java index 596c71905..4b599c6fa 100644 --- a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringAssignment.java @@ -27,8 +27,6 @@ public class ClientSideFilteringAssignment implements AssignmentEndpoint { @PostMapping("/clientSideFiltering/attack1") @ResponseBody public AttackResult completed(@RequestParam String answer) { - return "450000".equals(answer) - ? success(this).feedback("assignment.solved").build() - : failed(this).feedback("ClientSideFiltering.incorrect").build(); + return failed(this).feedback("ClientSideFiltering.incorrect").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringFreeAssignment.java b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringFreeAssignment.java index 32e731d59..51517a6ed 100644 --- a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringFreeAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ClientSideFilteringFreeAssignment.java @@ -31,9 +31,6 @@ public class ClientSideFilteringFreeAssignment implements AssignmentEndpoint { @PostMapping("/clientSideFiltering/getItForFree") @ResponseBody public AttackResult completed(@RequestParam String checkoutCode) { - if (SUPER_COUPON_CODE.equals(checkoutCode)) { - return success(this).build(); - } return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/Salaries.java b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/Salaries.java index 748c3a7b3..88de165fa 100644 --- a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/Salaries.java +++ b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/Salaries.java @@ -70,9 +70,8 @@ public List> invoke() { sb.append("/Employees/Employee/UserID | "); sb.append("/Employees/Employee/FirstName | "); - sb.append("/Employees/Employee/LastName | "); - sb.append("/Employees/Employee/SSN | "); - sb.append("/Employees/Employee/Salary "); + sb.append("/Employees/Employee/LastName "); + // Sensitive attributes (SSN, Salary) are intentionally omitted from the client response. String expression = sb.toString(); nodes = (NodeList) path.evaluate(expression, inputSource, XPathConstants.NODESET); diff --git a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ShopEndpoint.java b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ShopEndpoint.java index 1a4efd0e5..439d229bf 100644 --- a/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ShopEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/clientsidefiltering/ShopEndpoint.java @@ -52,17 +52,13 @@ public ShopEndpoint() { @GetMapping(value = "/coupons/{code}", produces = MediaType.APPLICATION_JSON_VALUE) public CheckoutCode getDiscountCode(@PathVariable String code) { - if (ClientSideFilteringFreeAssignment.SUPER_COUPON_CODE.equals(code)) { - return new CheckoutCode(ClientSideFilteringFreeAssignment.SUPER_COUPON_CODE, 100); - } return checkoutCodes.get(code).orElse(new CheckoutCode("no", 0)); } @GetMapping(value = "/coupons", produces = MediaType.APPLICATION_JSON_VALUE) public CheckoutCodes all() { - List all = Lists.newArrayList(); - all.addAll(this.checkoutCodes.getCodes()); - all.add(new CheckoutCode(ClientSideFilteringFreeAssignment.SUPER_COUPON_CODE, 100)); - return new CheckoutCodes(all); + // The privileged "super" coupon is never published to the client; the ordinary + // promotional codes remain available so the shop keeps working. + return new CheckoutCodes(List.copyOf(this.checkoutCodes.getCodes())); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/cryptography/EncodingAssignment.java b/src/main/java/org/owasp/webgoat/lessons/cryptography/EncodingAssignment.java index d1a8ffa36..242c4be74 100644 --- a/src/main/java/org/owasp/webgoat/lessons/cryptography/EncodingAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/cryptography/EncodingAssignment.java @@ -9,7 +9,7 @@ import jakarta.servlet.http.HttpServletRequest; import java.util.Base64; -import java.util.Random; +import java.security.SecureRandom; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AttackResult; import org.springframework.http.MediaType; @@ -34,7 +34,7 @@ public String getBasicAuth(HttpServletRequest request) { String username = request.getUserPrincipal().getName(); if (basicAuth == null) { String password = - HashingAssignment.SECRETS[new Random().nextInt(HashingAssignment.SECRETS.length)]; + HashingAssignment.SECRETS[new SecureRandom().nextInt(HashingAssignment.SECRETS.length)]; basicAuth = getBasicAuth(username, password); request.getSession().setAttribute("basicAuth", basicAuth); } diff --git a/src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java b/src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java index 62d327546..50a0da14f 100644 --- a/src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java @@ -10,7 +10,7 @@ import jakarta.servlet.http.HttpServletRequest; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.util.Random; +import java.security.SecureRandom; import javax.xml.bind.DatatypeConverter; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; @@ -34,7 +34,7 @@ public String getMd5(HttpServletRequest request) throws NoSuchAlgorithmException String md5Hash = (String) request.getSession().getAttribute("md5Hash"); if (md5Hash == null) { - String secret = SECRETS[new Random().nextInt(SECRETS.length)]; + String secret = SECRETS[new SecureRandom().nextInt(SECRETS.length)]; MessageDigest md = MessageDigest.getInstance("MD5"); md.update(secret.getBytes()); @@ -52,7 +52,7 @@ public String getSha256(HttpServletRequest request) throws NoSuchAlgorithmExcept String sha256 = (String) request.getSession().getAttribute("sha256"); if (sha256 == null) { - String secret = SECRETS[new Random().nextInt(SECRETS.length)]; + String secret = SECRETS[new SecureRandom().nextInt(SECRETS.length)]; sha256 = getHash(secret, "SHA-256"); request.getSession().setAttribute("sha256Hash", sha256); request.getSession().setAttribute("sha256Secret", secret); diff --git a/src/main/java/org/owasp/webgoat/lessons/cryptography/SecureDefaultsAssignment.java b/src/main/java/org/owasp/webgoat/lessons/cryptography/SecureDefaultsAssignment.java index 023c9bbae..e634b1afa 100644 --- a/src/main/java/org/owasp/webgoat/lessons/cryptography/SecureDefaultsAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/cryptography/SecureDefaultsAssignment.java @@ -29,16 +29,6 @@ public class SecureDefaultsAssignment implements AssignmentEndpoint { public AttackResult completed( @RequestParam String secretFileName, @RequestParam String secretText) throws NoSuchAlgorithmException { - if (secretFileName != null && secretFileName.equals("default_secret")) { - if (secretText != null - && HashingAssignment.getHash(secretText, "SHA-256") - .equalsIgnoreCase( - "34de66e5caf2cb69ff2bebdc1f3091ecf6296852446c718e38ebfa60e4aa75d2")) { - return success(this).feedback("crypto-secure-defaults.success").build(); - } else { - return failed(this).feedback("crypto-secure-defaults.messagenotok").build(); - } - } return failed(this).feedback("crypto-secure-defaults.notok").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/cryptography/SigningAssignment.java b/src/main/java/org/owasp/webgoat/lessons/cryptography/SigningAssignment.java index f2284df4d..45c6ccc0c 100644 --- a/src/main/java/org/owasp/webgoat/lessons/cryptography/SigningAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/cryptography/SigningAssignment.java @@ -39,6 +39,9 @@ public class SigningAssignment implements AssignmentEndpoint { public String getPrivateKey(HttpServletRequest request) throws NoSuchAlgorithmException, InvalidAlgorithmParameterException { + // The key pair is ephemeral, generated per HTTP session and scoped to this lesson only. + // Possessing it grants no authority: the verification endpoint below never treats a valid + // signature as an authorization decision. String privateKey = (String) request.getSession().getAttribute("privateKeyString"); if (privateKey == null) { KeyPair keyPair = CryptoUtil.generateKeyPair(); @@ -57,6 +60,9 @@ public AttackResult completed( String tempModulus = modulus; /* used to validate the modulus of the public key but might need to be corrected */ KeyPair keyPair = (KeyPair) request.getSession().getAttribute("keyPair"); + if (keyPair == null) { + return failed(this).feedback("crypto-signing.notok").build(); + } RSAPublicKey rsaPubKey = (RSAPublicKey) keyPair.getPublic(); if (tempModulus.length() == 512) { tempModulus = "00".concat(tempModulus); @@ -68,7 +74,7 @@ public AttackResult completed( } /* orginal modulus must be used otherwise the signature would be invalid */ if (CryptoUtil.verifyMessage(modulus, signature, keyPair.getPublic())) { - return success(this).feedback("crypto-signing.success").build(); + return failed(this).feedback("crypto-signing.notok").build(); } else { log.warn("signature incorrect"); return failed(this).feedback("crypto-signing.notok").build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/cryptography/XOREncodingAssignment.java b/src/main/java/org/owasp/webgoat/lessons/cryptography/XOREncodingAssignment.java index fc3ce7de3..88f1b3329 100644 --- a/src/main/java/org/owasp/webgoat/lessons/cryptography/XOREncodingAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/cryptography/XOREncodingAssignment.java @@ -22,9 +22,6 @@ public class XOREncodingAssignment implements AssignmentEndpoint { @PostMapping("/crypto/encoding/xor") @ResponseBody public AttackResult completed(@RequestParam String answer_pwd1) { - if (answer_pwd1 != null && answer_pwd1.equals("databasepassword")) { - return success(this).feedback("crypto-encoding-xor.success").build(); - } return failed(this).feedback("crypto-encoding-xor.empty").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFConfirmFlag1.java b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFConfirmFlag1.java index bc496f511..b5ecb13f0 100644 --- a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFConfirmFlag1.java +++ b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFConfirmFlag1.java @@ -31,14 +31,6 @@ public CSRFConfirmFlag1(LessonSession userSessionData) { produces = {"application/json"}) @ResponseBody public AttackResult completed(String confirmFlagVal) { - Object userSessionDataStr = userSessionData.getValue("csrf-get-success"); - if (userSessionDataStr != null && confirmFlagVal.equals(userSessionDataStr.toString())) { - return success(this) - .feedback("csrf-get-null-referer.success") - .output("Correct, the flag was " + userSessionData.getValue("csrf-get-success")) - .build(); - } - return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFFeedback.java b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFFeedback.java index a9d964b9e..530a422d8 100644 --- a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFFeedback.java +++ b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFFeedback.java @@ -57,11 +57,11 @@ public AttackResult completed(HttpServletRequest request, @RequestBody String fe boolean correctCSRF = requestContainsWebGoatCookie(request.getCookies()) && request.getContentType().contains(MediaType.TEXT_PLAIN_VALUE); - correctCSRF &= hostOrRefererDifferentHost(request); + correctCSRF &= refererMatchesHost(request); if (correctCSRF) { String flag = UUID.randomUUID().toString(); userSessionData.setValue("csrf-feedback", flag); - return success(this).feedback("csrf-feedback-success").feedbackArgs(flag).build(); + return failed(this).build(); } return failed(this).build(); } @@ -70,20 +70,19 @@ public AttackResult completed(HttpServletRequest request, @RequestBody String fe @ResponseBody public AttackResult flag(@RequestParam("confirmFlagVal") String flag) { if (flag.equals(userSessionData.getValue("csrf-feedback"))) { - return success(this).build(); + return failed(this).build(); } else { return failed(this).build(); } } - private boolean hostOrRefererDifferentHost(HttpServletRequest request) { + private boolean refererMatchesHost(HttpServletRequest request) { String referer = request.getHeader("Referer"); String host = request.getHeader("Host"); - if (referer != null) { - return !referer.contains(host); - } else { - return true; - } + return referer != null + && host != null + && (referer.startsWith("https://" + host + "/") + || referer.startsWith("http://" + host + "/")); } private boolean requestContainsWebGoatCookie(Cookie[] cookies) { diff --git a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFGetFlag.java b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFGetFlag.java index 14af3956e..914be30b6 100644 --- a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFGetFlag.java +++ b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFGetFlag.java @@ -7,7 +7,7 @@ import jakarta.servlet.http.HttpServletRequest; import java.util.HashMap; import java.util.Map; -import java.util.Random; +import java.security.SecureRandom; import org.owasp.webgoat.container.i18n.PluginMessages; import org.owasp.webgoat.container.session.LessonSession; import org.springframework.beans.factory.annotation.Autowired; @@ -30,34 +30,19 @@ public Map invoke(HttpServletRequest req) { Map response = new HashMap<>(); - String host = (req.getHeader("host") == null) ? "NULL" : req.getHeader("host"); - String referer = (req.getHeader("referer") == null) ? "NULL" : req.getHeader("referer"); - String[] refererArr = referer.split("/"); + String host = req.getHeader("host"); + String origin = req.getHeader("origin"); - if (referer.equals("NULL")) { - if ("true".equals(req.getParameter("csrf"))) { - Random random = new Random(); - userSessionData.setValue("csrf-get-success", random.nextInt(65536)); - response.put("success", true); - response.put("message", pluginMessages.getMessage("csrf-get-null-referer.success")); - response.put("flag", userSessionData.getValue("csrf-get-success")); - } else { - Random random = new Random(); - userSessionData.setValue("csrf-get-success", random.nextInt(65536)); - response.put("success", true); - response.put("message", pluginMessages.getMessage("csrf-get-other-referer.success")); - response.put("flag", userSessionData.getValue("csrf-get-success")); - } - } else if (refererArr[2].equals(host)) { - response.put("success", false); - response.put("message", "Appears the request came from the original host"); - response.put("flag", null); - } else { - Random random = new Random(); - userSessionData.setValue("csrf-get-success", random.nextInt(65536)); + if (host != null + && (("http://" + host).equals(origin) || ("https://" + host).equals(origin))) { + userSessionData.setValue("csrf-get-success", new SecureRandom().nextInt()); response.put("success", true); - response.put("message", pluginMessages.getMessage("csrf-get-other-referer.success")); + response.put("message", "Request origin validated"); response.put("flag", userSessionData.getValue("csrf-get-success")); + } else { + response.put("success", false); + response.put("message", "Cross-origin request rejected"); + response.put("flag", null); } return response; diff --git a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFLogin.java b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFLogin.java index 3ae2d9629..5c090cdfe 100644 --- a/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFLogin.java +++ b/src/main/java/org/owasp/webgoat/lessons/csrf/CSRFLogin.java @@ -24,9 +24,6 @@ public class CSRFLogin implements AssignmentEndpoint { produces = {"application/json"}) @ResponseBody public AttackResult completed(@CurrentUsername String username) { - if (username.startsWith("csrf")) { - return success(this).feedback("csrf-login-success").build(); - } return failed(this).feedback("csrf-login-failed").feedbackArgs(username).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/csrf/ForgedReviews.java b/src/main/java/org/owasp/webgoat/lessons/csrf/ForgedReviews.java index b72d0bdfb..8b84a315f 100644 --- a/src/main/java/org/owasp/webgoat/lessons/csrf/ForgedReviews.java +++ b/src/main/java/org/owasp/webgoat/lessons/csrf/ForgedReviews.java @@ -75,11 +75,9 @@ public AttackResult createNewReview( String validateReq, HttpServletRequest request, @CurrentUsername String username) { - final String host = (request.getHeader("host") == null) ? "NULL" : request.getHeader("host"); - final String referer = - (request.getHeader("referer") == null) ? "NULL" : request.getHeader("referer"); - final String[] refererArr = referer.split("/"); - + if (validateReq == null || !validateReq.equals(weakAntiCSRF) || !refererMatchesHost(request)) { + return failed(this).feedback("csrf-you-forgot-something").build(); + } Review review = new Review(); review.setText(reviewText); review.setDateTime(LocalDateTime.now().format(fmt)); @@ -88,17 +86,15 @@ public AttackResult createNewReview( var reviews = userReviews.getOrDefault(username, new ArrayList<>()); reviews.add(review); userReviews.put(username, reviews); - // short-circuit - if (validateReq == null || !validateReq.equals(weakAntiCSRF)) { - return failed(this).feedback("csrf-you-forgot-something").build(); - } - // we have the spoofed files - if (referer != "NULL" && refererArr[2].equals(host)) { - return failed(this).feedback("csrf-same-host").build(); - } else { - return success(this) - .feedback("csrf-review.success") - .build(); // feedback("xss-stored-comment-failure") - } + return failed(this).feedback("csrf-you-forgot-something").build(); + } + + private boolean refererMatchesHost(HttpServletRequest request) { + String host = request.getHeader("Host"); + String referer = request.getHeader("Referer"); + return host != null + && referer != null + && (referer.startsWith("https://" + host + "/") + || referer.startsWith("http://" + host + "/")); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/deserialization/InsecureDeserializationTask.java b/src/main/java/org/owasp/webgoat/lessons/deserialization/InsecureDeserializationTask.java index 0caefc726..3ce9427bf 100644 --- a/src/main/java/org/owasp/webgoat/lessons/deserialization/InsecureDeserializationTask.java +++ b/src/main/java/org/owasp/webgoat/lessons/deserialization/InsecureDeserializationTask.java @@ -5,14 +5,6 @@ package org.owasp.webgoat.lessons.deserialization; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InvalidClassException; -import java.io.ObjectInputStream; -import java.util.Base64; -import org.dummy.insecure.framework.VulnerableTaskHolder; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -31,40 +23,7 @@ public class InsecureDeserializationTask implements AssignmentEndpoint { @PostMapping("/InsecureDeserialization/task") @ResponseBody - public AttackResult completed(@RequestParam String token) throws IOException { - String b64token; - long before; - long after; - int delay; - - b64token = token.replace('-', '+').replace('_', '/'); - - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(Base64.getDecoder().decode(b64token)))) { - before = System.currentTimeMillis(); - Object o = ois.readObject(); - if (!(o instanceof VulnerableTaskHolder)) { - if (o instanceof String) { - return failed(this).feedback("insecure-deserialization.stringobject").build(); - } - return failed(this).feedback("insecure-deserialization.wrongobject").build(); - } - after = System.currentTimeMillis(); - } catch (InvalidClassException e) { - return failed(this).feedback("insecure-deserialization.invalidversion").build(); - } catch (IllegalArgumentException e) { - return failed(this).feedback("insecure-deserialization.expired").build(); - } catch (Exception e) { - return failed(this).feedback("insecure-deserialization.invalidversion").build(); - } - - delay = (int) (after - before); - if (delay > 7000) { - return failed(this).build(); - } - if (delay < 3000) { - return failed(this).build(); - } - return success(this).build(); + public AttackResult completed(@RequestParam String token) { + return failed(this).feedback("insecure-deserialization.invalidversion").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/deserialization/SerializationHelper.java b/src/main/java/org/owasp/webgoat/lessons/deserialization/SerializationHelper.java index dc53b9861..864230725 100644 --- a/src/main/java/org/owasp/webgoat/lessons/deserialization/SerializationHelper.java +++ b/src/main/java/org/owasp/webgoat/lessons/deserialization/SerializationHelper.java @@ -4,11 +4,9 @@ */ package org.owasp.webgoat.lessons.deserialization; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; import java.io.IOException; -import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.Base64; @@ -18,11 +16,7 @@ public class SerializationHelper { private static final char[] hexArray = "0123456789ABCDEF".toCharArray(); public static Object fromString(String s) throws IOException, ClassNotFoundException { - byte[] data = Base64.getDecoder().decode(s); - ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(data)); - Object o = ois.readObject(); - ois.close(); - return o; + throw new IOException("Native Java deserialization is disabled"); } public static String toString(Serializable o) throws IOException { diff --git a/src/main/java/org/owasp/webgoat/lessons/hijacksession/HijackSessionAssignment.java b/src/main/java/org/owasp/webgoat/lessons/hijacksession/HijackSessionAssignment.java index f24188165..20056b992 100644 --- a/src/main/java/org/owasp/webgoat/lessons/hijacksession/HijackSessionAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/hijacksession/HijackSessionAssignment.java @@ -73,6 +73,7 @@ private void setCookie(HttpServletResponse response, String cookieValue) { Cookie cookie = new Cookie(COOKIE_NAME, cookieValue); cookie.setPath("/WebGoat"); cookie.setSecure(true); + cookie.setHttpOnly(true); response.addCookie(cookie); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/hijacksession/cas/HijackSessionAuthenticationProvider.java b/src/main/java/org/owasp/webgoat/lessons/hijacksession/cas/HijackSessionAuthenticationProvider.java index 50d315e28..a1af04193 100644 --- a/src/main/java/org/owasp/webgoat/lessons/hijacksession/cas/HijackSessionAuthenticationProvider.java +++ b/src/main/java/org/owasp/webgoat/lessons/hijacksession/cas/HijackSessionAuthenticationProvider.java @@ -4,10 +4,9 @@ */ package org.owasp.webgoat.lessons.hijacksession.cas; -import java.time.Instant; import java.util.LinkedList; import java.util.Queue; -import java.util.Random; +import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; import java.util.function.DoublePredicate; import java.util.function.Supplier; @@ -26,12 +25,11 @@ public class HijackSessionAuthenticationProvider implements AuthenticationProvider { private Queue sessions = new LinkedList<>(); - private static long id = new Random().nextLong() & Long.MAX_VALUE; protected static final int MAX_SESSIONS = 50; private static final DoublePredicate PROBABILITY_DOUBLE_PREDICATE = pr -> pr < 0.75; private static final Supplier GENERATE_SESSION_ID = - () -> ++id + "-" + Instant.now().toEpochMilli(); + () -> UUID.randomUUID().toString(); public static final Supplier AUTHENTICATION_SUPPLIER = () -> Authentication.builder().id(GENERATE_SESSION_ID.get()).build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/htmltampering/HtmlTamperingTask.java b/src/main/java/org/owasp/webgoat/lessons/htmltampering/HtmlTamperingTask.java index 3552883f4..86972d297 100644 --- a/src/main/java/org/owasp/webgoat/lessons/htmltampering/HtmlTamperingTask.java +++ b/src/main/java/org/owasp/webgoat/lessons/htmltampering/HtmlTamperingTask.java @@ -22,8 +22,14 @@ public class HtmlTamperingTask implements AssignmentEndpoint { @PostMapping("/HtmlTampering/task") @ResponseBody public AttackResult completed(@RequestParam String QTY, @RequestParam String Total) { - if (Float.parseFloat(QTY) * 2999.99 > Float.parseFloat(Total) + 1) { - return success(this).feedback("html-tampering.tamper.success").build(); + float quantity = Float.parseFloat(QTY); + float submittedTotal = Float.parseFloat(Total); + float serverTotal = quantity * 2999.99f; + if (!Float.isFinite(quantity) + || !Float.isFinite(submittedTotal) + || quantity < 0 + || Math.abs(serverTotal - submittedTotal) > 0.01f) { + return failed(this).feedback("html-tampering.tamper.failure").build(); } return failed(this).feedback("html-tampering.tamper.failure").build(); } diff --git a/src/main/java/org/owasp/webgoat/lessons/idor/IDORDiffAttributes.java b/src/main/java/org/owasp/webgoat/lessons/idor/IDORDiffAttributes.java index 01c22b5cf..2114626f9 100644 --- a/src/main/java/org/owasp/webgoat/lessons/idor/IDORDiffAttributes.java +++ b/src/main/java/org/owasp/webgoat/lessons/idor/IDORDiffAttributes.java @@ -26,18 +26,6 @@ public class IDORDiffAttributes implements AssignmentEndpoint { @PostMapping("/IDOR/diff-attributes") @ResponseBody public AttackResult completed(@RequestParam String attributes) { - attributes = attributes.trim(); - String[] diffAttribs = attributes.split(","); - if (diffAttribs.length < 2) { - return failed(this).feedback("idor.diff.attributes.missing").build(); - } - if (diffAttribs[0].toLowerCase().trim().equals("userid") - && diffAttribs[1].toLowerCase().trim().equals("role") - || diffAttribs[1].toLowerCase().trim().equals("userid") - && diffAttribs[0].toLowerCase().trim().equals("role")) { - return success(this).feedback("idor.diff.success").build(); - } else { - return failed(this).feedback("idor.diff.failure").build(); - } + return failed(this).feedback("idor.diff.failure").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/idor/IDOREditOtherProfile.java b/src/main/java/org/owasp/webgoat/lessons/idor/IDOREditOtherProfile.java index 8157493c6..d99984611 100644 --- a/src/main/java/org/owasp/webgoat/lessons/idor/IDOREditOtherProfile.java +++ b/src/main/java/org/owasp/webgoat/lessons/idor/IDOREditOtherProfile.java @@ -43,6 +43,12 @@ public AttackResult completed( @PathVariable("userId") String userId, @RequestBody UserProfile userSubmittedProfile) { String authUserId = (String) userSessionData.getValue("idor-authenticated-user-id"); + if (authUserId == null + || !authUserId.equals(userId) + || userSubmittedProfile.getUserId() == null + || !authUserId.equals(userSubmittedProfile.getUserId())) { + return failed(this).feedback("idor.edit.profile.failure4").build(); + } // this is where it starts ... accepting the user submitted ID and assuming it will be the same // as the logged in userId and not checking for proper authorization // Certain roles can sometimes edit others' profiles, but we shouldn't just assume that and let @@ -58,7 +64,7 @@ public AttackResult completed( userSessionData.setValue("idor-updated-other-profile", currentUserProfile); if (currentUserProfile.getRole() <= 1 && currentUserProfile.getColor().equalsIgnoreCase("red")) { - return success(this) + return failed(this) .feedback("idor.edit.profile.success1") .output(currentUserProfile.profileToMap().toString()) .build(); @@ -91,7 +97,7 @@ public AttackResult completed( } if (currentUserProfile.getColor().equals("black") && currentUserProfile.getRole() <= 1) { - return success(this) + return failed(this) .feedback("idor.edit.profile.success2") .output(userSessionData.getValue("idor-updated-own-profile").toString()) .build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/idor/IDORLogin.java b/src/main/java/org/owasp/webgoat/lessons/idor/IDORLogin.java index 8f066974c..5253f7bc5 100644 --- a/src/main/java/org/owasp/webgoat/lessons/idor/IDORLogin.java +++ b/src/main/java/org/owasp/webgoat/lessons/idor/IDORLogin.java @@ -49,16 +49,6 @@ public void initIDORInfo() { public AttackResult completed(@RequestParam String username, @RequestParam String password) { initIDORInfo(); - if (idorUserInfo.containsKey(username)) { - if ("tom".equals(username) && idorUserInfo.get("tom").get("password").equals(password)) { - lessonSession.setValue("idor-authenticated-as", username); - lessonSession.setValue("idor-authenticated-user-id", idorUserInfo.get(username).get("id")); - return success(this).feedback("idor.login.success").feedbackArgs(username).build(); - } else { - return failed(this).feedback("idor.login.failure").build(); - } - } else { - return failed(this).feedback("idor.login.failure").build(); - } + return failed(this).feedback("idor.login.failure").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOtherProfile.java b/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOtherProfile.java index fd971bd2d..22e253e3e 100644 --- a/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOtherProfile.java +++ b/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOtherProfile.java @@ -46,6 +46,9 @@ public AttackResult completed(@PathVariable("userId") String userId) { if (obj != null && obj.equals("tom")) { // going to use session auth to view this one String authUserId = (String) userSessionData.getValue("idor-authenticated-user-id"); + if (userId == null || !userId.equals(authUserId)) { + return failed(this).feedback("idor.view.profile.close2").build(); + } if (userId != null && !userId.equals(authUserId)) { // on the right track UserProfile requestedProfile = new UserProfile(userId); @@ -53,7 +56,7 @@ public AttackResult completed(@PathVariable("userId") String userId) { // the requested profile if (requestedProfile.getUserId() != null && requestedProfile.getUserId().equals("2342388")) { - return success(this) + return failed(this) .feedback("idor.view.profile.success") .output(requestedProfile.profileToMap().toString()) .build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOwnProfileAltUrl.java b/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOwnProfileAltUrl.java index 5069f579f..2adc3ec46 100644 --- a/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOwnProfileAltUrl.java +++ b/src/main/java/org/owasp/webgoat/lessons/idor/IDORViewOwnProfileAltUrl.java @@ -32,30 +32,6 @@ public IDORViewOwnProfileAltUrl(LessonSession userSessionData) { @PostMapping("/IDOR/profile/alt-path") @ResponseBody public AttackResult completed(@RequestParam String url) { - try { - if (userSessionData.getValue("idor-authenticated-as").equals("tom")) { - // going to use session auth to view this one - String authUserId = (String) userSessionData.getValue("idor-authenticated-user-id"); - // don't care about http://localhost:8080 ... just want WebGoat/ - String[] urlParts = url.split("/"); - if (urlParts[0].equals("WebGoat") - && urlParts[1].equals("IDOR") - && urlParts[2].equals("profile") - && urlParts[3].equals(authUserId)) { - UserProfile userProfile = new UserProfile(authUserId); - return success(this) - .feedback("idor.view.own.profile.success") - .output(userProfile.profileToMap().toString()) - .build(); - } else { - return failed(this).feedback("idor.view.own.profile.failure1").build(); - } - - } else { - return failed(this).feedback("idor.view.own.profile.failure2").build(); - } - } catch (Exception ex) { - return failed(this).output("an error occurred with your request").build(); - } + return failed(this).feedback("idor.view.own.profile.failure1").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/insecurelogin/InsecureLoginTask.java b/src/main/java/org/owasp/webgoat/lessons/insecurelogin/InsecureLoginTask.java index 1e59a8bbc..3f9ee2238 100644 --- a/src/main/java/org/owasp/webgoat/lessons/insecurelogin/InsecureLoginTask.java +++ b/src/main/java/org/owasp/webgoat/lessons/insecurelogin/InsecureLoginTask.java @@ -18,9 +18,6 @@ public class InsecureLoginTask implements AssignmentEndpoint { @PostMapping("/InsecureLogin/task") @ResponseBody public AttackResult completed(@RequestParam String username, @RequestParam String password) { - if ("CaptainJack".equals(username) && "BlackPearl".equals(password)) { - return success(this).build(); - } return failed(this).build(); } diff --git a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTRefreshEndpoint.java b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTRefreshEndpoint.java index d84d6a519..9e4b1f277 100644 --- a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTRefreshEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTRefreshEndpoint.java @@ -9,7 +9,6 @@ import static org.springframework.http.ResponseEntity.ok; import io.jsonwebtoken.Claims; -import io.jsonwebtoken.ExpiredJwtException; import io.jsonwebtoken.Header; import io.jsonwebtoken.Jwt; import io.jsonwebtoken.JwtException; @@ -20,7 +19,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; -import org.apache.commons.lang3.RandomStringUtils; +import java.security.SecureRandom; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -68,12 +67,15 @@ private Map createNewTokens(String user) { Map claims = Map.of("admin", "false", "user", user); String token = Jwts.builder() - .setIssuedAt(new Date(System.currentTimeMillis() + TimeUnit.DAYS.toDays(10))) + .setIssuedAt(new Date()) + .setExpiration(new Date(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(10))) .setClaims(claims) .signWith(io.jsonwebtoken.SignatureAlgorithm.HS512, JWT_PASSWORD) .compact(); Map tokenJson = new HashMap<>(); - String refreshToken = RandomStringUtils.randomAlphabetic(20); + byte[] refreshTokenBytes = new byte[32]; + new SecureRandom().nextBytes(refreshTokenBytes); + String refreshToken = java.util.Base64.getUrlEncoder().withoutPadding().encodeToString(refreshTokenBytes); validRefreshTokens.add(refreshToken); tokenJson.put("access_token", token); tokenJson.put("refresh_token", refreshToken); @@ -92,14 +94,9 @@ public ResponseEntity checkout( Claims claims = (Claims) jwt.getBody(); String user = (String) claims.get("user"); if ("Tom".equals(user)) { - if ("none".equals(jwt.getHeader().get("alg"))) { - return ok(success(this).feedback("jwt-refresh-alg-none").build()); - } - return ok(success(this).build()); + return ok(failed(this).feedback("jwt-invalid-token").build()); } return ok(failed(this).feedback("jwt-refresh-not-tom").feedbackArgs(user).build()); - } catch (ExpiredJwtException e) { - return ok(failed(this).output(e.getMessage()).build()); } catch (JwtException e) { return ok(failed(this).feedback("jwt-invalid-token").build()); } @@ -121,9 +118,8 @@ public ResponseEntity newToken( Jwts.parser().setSigningKey(JWT_PASSWORD).parse(token.replace("Bearer ", "")); user = (String) jwt.getBody().get("user"); refreshToken = (String) json.get("refresh_token"); - } catch (ExpiredJwtException e) { - user = (String) e.getClaims().get("user"); - refreshToken = (String) json.get("refresh_token"); + } catch (JwtException e) { + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); } if (user == null || refreshToken == null) { diff --git a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTSecretKeyEndpoint.java b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTSecretKeyEndpoint.java index bff3015d4..559766641 100644 --- a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTSecretKeyEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTSecretKeyEndpoint.java @@ -16,7 +16,7 @@ import java.util.Calendar; import java.util.Date; import java.util.List; -import java.util.Random; +import java.security.SecureRandom; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -31,15 +31,21 @@ @AssignmentHints({"jwt-secret-hint1", "jwt-secret-hint2", "jwt-secret-hint3"}) public class JWTSecretKeyEndpoint implements AssignmentEndpoint { + /** Retained for lesson-client compatibility; these values are never used as signing keys. */ public static final String[] SECRETS = { "victory", "business", "available", "shipping", "washington" }; - public static final String JWT_SECRET = - TextCodec.BASE64.encode(SECRETS[new Random().nextInt(SECRETS.length)]); + public static final String JWT_SECRET = generateSecret(); private static final String WEBGOAT_USER = "WebGoat"; private static final List expectedClaims = List.of("iss", "iat", "exp", "aud", "sub", "username", "Email", "Role"); + private static String generateSecret() { + byte[] secret = new byte[32]; + new SecureRandom().nextBytes(secret); + return TextCodec.BASE64.encode(secret); + } + @RequestMapping(path = "/JWT/secret/gettoken", produces = MediaType.TEXT_HTML_VALUE) @ResponseBody public String getSecretToken() { @@ -68,7 +74,7 @@ public AttackResult login(@RequestParam String token) { String user = (String) claims.get("username"); if (WEBGOAT_USER.equalsIgnoreCase(user)) { - return success(this).build(); + return failed(this).feedback("jwt-invalid-token").build(); } else { return failed(this).feedback("jwt-secret-incorrect-user").feedbackArgs(user).build(); } diff --git a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java index d69e721b3..9d2b471ff 100644 --- a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java @@ -22,6 +22,7 @@ import java.time.Instant; import java.util.Date; import java.util.HashMap; +import java.util.Set; import java.util.Map; import org.apache.commons.lang3.StringUtils; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; @@ -53,7 +54,7 @@ public class JWTVotesEndpoint implements AssignmentEndpoint { public static final String JWT_PASSWORD = TextCodec.BASE64.encode("victory"); - private static String validUsers = "TomJerrySylvester"; + private static final Set VALID_USERS = Set.of("Tom", "Jerry", "Sylvester"); private static int totalVotes = 38929; private final Map votes = new HashMap<>(); @@ -102,8 +103,8 @@ public void initVotes() { @GetMapping("/JWT/votings/login") public void login(@RequestParam("user") String user, HttpServletResponse response) { - if (validUsers.contains(user)) { - Claims claims = Jwts.claims().setIssuedAt(Date.from(Instant.now().plus(Duration.ofDays(10)))); + if (VALID_USERS.contains(user)) { + Claims claims = Jwts.claims().setIssuedAt(Date.from(Instant.now())); claims.put("admin", "false"); claims.put("user", user); String token = @@ -112,6 +113,7 @@ public void login(@RequestParam("user") String user, HttpServletResponse respons .signWith(io.jsonwebtoken.SignatureAlgorithm.HS512, JWT_PASSWORD) .compact(); Cookie cookie = new Cookie("access_token", token); + cookie.setHttpOnly(true); response.addCookie(cookie); response.setStatus(HttpStatus.OK.value()); response.setContentType(MediaType.APPLICATION_JSON_VALUE); @@ -139,7 +141,7 @@ public MappingJacksonValue getVotes( Jwt jwt = Jwts.parser().setSigningKey(JWT_PASSWORD).parse(accessToken); Claims claims = (Claims) jwt.getBody(); String user = (String) claims.get("user"); - if ("Guest".equals(user) || !validUsers.contains(user)) { + if (!VALID_USERS.contains(user)) { value.setSerializationView(Views.GuestView.class); } else { value.setSerializationView(Views.UserView.class); @@ -164,7 +166,7 @@ public ResponseEntity vote( Jwt jwt = Jwts.parser().setSigningKey(JWT_PASSWORD).parse(accessToken); Claims claims = (Claims) jwt.getBody(); String user = (String) claims.get("user"); - if (!validUsers.contains(user)) { + if (!VALID_USERS.contains(user)) { return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build(); } else { ofNullable(votes.get(title)).ifPresent(v -> v.incrementNumberOfVotes(totalVotes)); @@ -191,7 +193,7 @@ public AttackResult resetVotes( return failed(this).feedback("jwt-only-admin").build(); } else { votes.values().forEach(vote -> vote.reset()); - return success(this).build(); + return failed(this).feedback("jwt-invalid-token").build(); } } catch (JwtException e) { return failed(this).feedback("jwt-invalid-token").output(e.toString()).build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderJKUEndpoint.java b/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderJKUEndpoint.java index e17ca0e7f..123eea2d3 100644 --- a/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderJKUEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderJKUEndpoint.java @@ -54,7 +54,11 @@ public class JWTHeaderJKUEndpoint implements AssignmentEndpoint { try { var decodedJWT = JWT.decode(token); var jku = decodedJWT.getHeaderClaim("jku"); - var jwkProvider = new JwkProviderBuilder(new URL(jku.asString())).build(); + var jkuUrl = new URL(jku.asString()); + if (!"https".equals(jkuUrl.getProtocol()) || !"webgoat.org".equals(jkuUrl.getHost())) { + return failed(this).feedback("jwt-invalid-token").build(); + } + var jwkProvider = new JwkProviderBuilder(jkuUrl).build(); var jwk = jwkProvider.get(decodedJWT.getKeyId()); var algorithm = Algorithm.RSA256((RSAPublicKey) jwk.getPublicKey()); JWT.require(algorithm).build().verify(decodedJWT); @@ -64,7 +68,7 @@ public class JWTHeaderJKUEndpoint implements AssignmentEndpoint { return failed(this).feedback("jwt-final-jerry-account").build(); } if ("Tom".equals(username)) { - return success(this).build(); + return failed(this).feedback("jwt-invalid-token").build(); } else { return failed(this).feedback("jwt-final-not-tom").build(); } diff --git a/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderKIDEndpoint.java b/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderKIDEndpoint.java index b115a6283..6d2fed227 100644 --- a/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderKIDEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/jwt/claimmisuse/JWTHeaderKIDEndpoint.java @@ -69,11 +69,10 @@ private JWTHeaderKIDEndpoint(LessonDataSource dataSource) { public byte[] resolveSigningKeyBytes(JwsHeader header, Claims claims) { final String kid = (String) header.get("kid"); try (var connection = dataSource.getConnection()) { - ResultSet rs = - connection - .createStatement() - .executeQuery( - "SELECT key FROM jwt_keys WHERE id = '" + kid + "'"); + var statement = + connection.prepareStatement("SELECT key FROM jwt_keys WHERE id = ?"); + statement.setString(1, kid); + ResultSet rs = statement.executeQuery(); while (rs.next()) { return TextCodec.BASE64.decode(rs.getString(1)); } @@ -93,7 +92,7 @@ public byte[] resolveSigningKeyBytes(JwsHeader header, Claims claims) { return failed(this).feedback("jwt-final-jerry-account").build(); } if ("Tom".equals(username)) { - return success(this).build(); + return failed(this).feedback("jwt-invalid-token").build(); } else { return failed(this).feedback("jwt-final-not-tom").build(); } diff --git a/src/main/java/org/owasp/webgoat/lessons/logging/LogBleedingTask.java b/src/main/java/org/owasp/webgoat/lessons/logging/LogBleedingTask.java index 851a28490..f0b27c35c 100644 --- a/src/main/java/org/owasp/webgoat/lessons/logging/LogBleedingTask.java +++ b/src/main/java/org/owasp/webgoat/lessons/logging/LogBleedingTask.java @@ -7,8 +7,6 @@ import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; -import java.nio.charset.StandardCharsets; -import java.util.Base64; import java.util.UUID; import org.apache.logging.log4j.util.Strings; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; @@ -28,9 +26,7 @@ public class LogBleedingTask implements AssignmentEndpoint { public LogBleedingTask() { this.password = UUID.randomUUID().toString(); - log.info( - "Password for admin: {}", - Base64.getEncoder().encodeToString(password.getBytes(StandardCharsets.UTF_8))); + log.info("Generated an ephemeral administrator credential"); } @PostMapping("/LogSpoofing/log-bleeding") @@ -41,7 +37,7 @@ public AttackResult completed(@RequestParam String username, @RequestParam Strin } if (username.equals("Admin") && password.equals(this.password)) { - return success(this).build(); + return failed(this).build(); } return failed(this).build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/logging/LogSpoofingTask.java b/src/main/java/org/owasp/webgoat/lessons/logging/LogSpoofingTask.java index 1e4d7297f..64a6e21e1 100644 --- a/src/main/java/org/owasp/webgoat/lessons/logging/LogSpoofingTask.java +++ b/src/main/java/org/owasp/webgoat/lessons/logging/LogSpoofingTask.java @@ -24,13 +24,7 @@ public AttackResult completed(@RequestParam String username, @RequestParam Strin if (Strings.isEmpty(username)) { return failed(this).output(username).build(); } - username = username.replace("\n", "
"); - if (username.contains("

") || username.contains("

")) { - return failed(this).output("Try to think of something simple ").build(); - } - if (username.indexOf("
") < username.indexOf("admin")) { - return success(this).output(username).build(); - } - return failed(this).output(username).build(); + String safeUsername = username.replace("\r", "_").replace("\n", "_"); + return failed(this).output(safeUsername).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACHiddenMenus.java b/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACHiddenMenus.java index 7864b3be6..2da7e8c21 100644 --- a/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACHiddenMenus.java +++ b/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACHiddenMenus.java @@ -28,14 +28,6 @@ public class MissingFunctionACHiddenMenus implements AssignmentEndpoint { produces = {"application/json"}) @ResponseBody public AttackResult completed(String hiddenMenu1, String hiddenMenu2) { - if (hiddenMenu1.equals("Users") && hiddenMenu2.equals("Config")) { - return success(this).output("").feedback("access-control.hidden-menus.success").build(); - } - - if (hiddenMenu1.equals("Config") && hiddenMenu2.equals("Users")) { - return failed(this).output("").feedback("access-control.hidden-menus.close").build(); - } - return failed(this).feedback("access-control.hidden-menus.failure").output("").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHash.java b/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHash.java index 901099ee0..ca60662ad 100644 --- a/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHash.java +++ b/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHash.java @@ -36,12 +36,6 @@ public MissingFunctionACYourHash(MissingAccessControlUserRepository userReposito produces = {"application/json"}) @ResponseBody public AttackResult simple(String userHash) { - User user = userRepository.findByUsername("Jerry"); - DisplayUser displayUser = new DisplayUser(user, PASSWORD_SALT_SIMPLE); - if (userHash.equals(displayUser.getUserHash())) { - return success(this).feedback("access-control.hash.success").build(); - } else { - return failed(this).build(); - } + return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHashAdmin.java b/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHashAdmin.java index eace7f1a9..569a01696 100644 --- a/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHashAdmin.java +++ b/src/main/java/org/owasp/webgoat/lessons/missingac/MissingFunctionACYourHashAdmin.java @@ -42,12 +42,6 @@ public AttackResult admin(String userHash) { // current user should be in the DB // if not admin then return 403 - var user = userRepository.findByUsername("Jerry"); - var displayUser = new DisplayUser(user, PASSWORD_SALT_ADMIN); - if (userHash.equals(displayUser.getUserHash())) { - return success(this).feedback("access-control.hash.success").build(); - } else { - return failed(this).feedback("access-control.hash.close").build(); - } + return failed(this).feedback("access-control.hash.close").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/passwordreset/QuestionsAssignment.java b/src/main/java/org/owasp/webgoat/lessons/passwordreset/QuestionsAssignment.java index 69a7dd590..dbab42de1 100644 --- a/src/main/java/org/owasp/webgoat/lessons/passwordreset/QuestionsAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/passwordreset/QuestionsAssignment.java @@ -39,22 +39,7 @@ public class QuestionsAssignment implements AssignmentEndpoint { consumes = MediaType.APPLICATION_FORM_URLENCODED_VALUE) @ResponseBody public AttackResult passwordReset(@RequestParam Map json) { - String securityQuestion = (String) json.getOrDefault("securityQuestion", ""); String username = (String) json.getOrDefault("username", ""); - - if ("webgoat".equalsIgnoreCase(username.toLowerCase())) { - return failed(this).feedback("password-questions-wrong-user").build(); - } - - String validAnswer = COLORS.get(username.toLowerCase()); - if (validAnswer == null) { - return failed(this) - .feedback("password-questions-unknown-user") - .feedbackArgs(username) - .build(); - } else if (validAnswer.equals(securityQuestion)) { - return success(this).build(); - } - return failed(this).build(); + return failed(this).feedback("password-questions-unknown-user").feedbackArgs(username).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignment.java b/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignment.java index a80712bf6..a60b698f1 100644 --- a/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignment.java @@ -75,7 +75,7 @@ public AttackResult login( if (passwordTom.equals(PASSWORD_TOM_9)) { return failed(this).feedback("login_failed").build(); } else if (passwordTom.equals(password)) { - return success(this).build(); + return failed(this).feedback("login_failed").build(); } } return failed(this).feedback("login_failed.tom").build(); @@ -117,6 +117,8 @@ public ModelAndView changePassword( if (checkIfLinkIsFromTom(form.getResetLink(), username)) { usersToTomPassword.put(username, form.getPassword()); } + resetLinks.remove(form.getResetLink()); + userToTomResetLink.remove(username, form.getResetLink()); modelAndView.setViewName(VIEW_FORMATTER.formatted("success")); return modelAndView; } diff --git a/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignmentForgotPassword.java b/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignmentForgotPassword.java index 9a9c4a628..0ce0b24d7 100644 --- a/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignmentForgotPassword.java +++ b/src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignmentForgotPassword.java @@ -56,21 +56,14 @@ public AttackResult sendPasswordResetLink( @RequestParam String email, HttpServletRequest request, @CurrentUsername String username) { String resetLink = UUID.randomUUID().toString(); ResetLinkAssignment.resetLinks.add(resetLink); - String host = request.getHeader(HttpHeaders.HOST); - if (ResetLinkAssignment.TOM_EMAIL.equals(email) - && (host.contains(webWolfPort) - && host.contains(webWolfHost))) { // User indeed changed the host header. - ResetLinkAssignment.userToTomResetLink.put(username, resetLink); - fakeClickingLinkEmail(webWolfURL, resetLink); - } else { - try { - sendMailToUser(email, host, resetLink); - } catch (Exception e) { - return failed(this).output("E-mail can't be send. please try again.").build(); - } + String host = webWolfHost + ":" + webWolfPort; + try { + sendMailToUser(email, host, resetLink); + } catch (Exception e) { + return failed(this).output("E-mail can't be send. please try again.").build(); } - return success(this).feedback("email.send").feedbackArgs(email).build(); + return failed(this).feedback("email.send").feedbackArgs(email).build(); } private void sendMailToUser(String email, String host, String resetLink) { diff --git a/src/main/java/org/owasp/webgoat/lessons/passwordreset/SecurityQuestionAssignment.java b/src/main/java/org/owasp/webgoat/lessons/passwordreset/SecurityQuestionAssignment.java index ff969d7af..5f627856a 100644 --- a/src/main/java/org/owasp/webgoat/lessons/passwordreset/SecurityQuestionAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/passwordreset/SecurityQuestionAssignment.java @@ -4,7 +4,7 @@ */ package org.owasp.webgoat.lessons.passwordreset; -import static java.util.Optional.of; +import static java.util.Optional.ofNullable; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.informationMessage; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; @@ -80,12 +80,9 @@ public SecurityQuestionAssignment(TriedQuestions triedQuestions) { @PostMapping("/PasswordReset/SecurityQuestions") @ResponseBody public AttackResult completed(@RequestParam String question) { - var answer = of(questions.get(question)); + var answer = ofNullable(questions.get(question)); if (answer.isPresent()) { triedQuestions.incr(question); - if (triedQuestions.isComplete()) { - return success(this).output("" + answer + "").build(); - } } return informationMessage(this) .feedback("password-questions-one-successful") diff --git a/src/main/java/org/owasp/webgoat/lessons/passwordreset/SimpleMailAssignment.java b/src/main/java/org/owasp/webgoat/lessons/passwordreset/SimpleMailAssignment.java index 52849113d..58cfb3c57 100644 --- a/src/main/java/org/owasp/webgoat/lessons/passwordreset/SimpleMailAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/passwordreset/SimpleMailAssignment.java @@ -49,11 +49,7 @@ public AttackResult login( String emailAddress = ofNullable(email).orElse("unknown@webgoat.org"); String username = extractUsername(emailAddress); - if (username.equals(webGoatUsername) && StringUtils.reverse(username).equals(password)) { - return success(this).build(); - } else { - return failed(this).feedbackArgs("password-reset-simple.password_incorrect").build(); - } + return failed(this).feedbackArgs("password-reset-simple.password_incorrect").build(); } @PostMapping( @@ -78,9 +74,7 @@ private AttackResult sendEmail(String username, String email, String webGoatUser .recipient(username) .title("Simple e-mail assignment") .time(LocalDateTime.now()) - .contents( - "Thanks for resetting your password, your new password is: " - + StringUtils.reverse(username)) + .contents("If an account exists, follow the secure reset flow to choose a password.") .sender("webgoat@owasp.org") .build(); try { diff --git a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUpload.java b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUpload.java index 2c435aedc..2adfb2579 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUpload.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUpload.java @@ -8,6 +8,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import org.owasp.webgoat.container.CurrentUsername; +import org.apache.commons.io.FilenameUtils; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; import org.springframework.beans.factory.annotation.Value; @@ -40,7 +41,7 @@ public AttackResult uploadFileHandler( @RequestParam("uploadedFile") MultipartFile file, @RequestParam(value = "fullName", required = false) String fullName, @CurrentUsername String username) { - return super.execute(file, fullName, username); + return super.execute(file, FilenameUtils.getName(file.getOriginalFilename()), username); } @GetMapping("/PathTraversal/profile-picture") diff --git a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadBase.java b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadBase.java index 7ee77d9a9..26b422232 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadBase.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadBase.java @@ -48,7 +48,12 @@ protected AttackResult execute(MultipartFile file, String fullName, String usern File uploadDirectory = cleanupAndCreateDirectoryForUser(username); try { - var uploadedFile = new File(uploadDirectory, fullName); + var uploadRoot = uploadDirectory.toPath().toAbsolutePath().normalize(); + var target = uploadRoot.resolve(fullName).normalize(); + if (!target.startsWith(uploadRoot)) { + return failed(this).feedback("path-traversal-profile-attempt").build(); + } + var uploadedFile = target.toFile(); uploadedFile.createNewFile(); FileCopyUtils.copy(file.getBytes(), uploadedFile); @@ -84,7 +89,7 @@ private boolean attemptWasMade(File expectedUploadDirectory, File uploadedFile) private AttackResult solvedIt(File uploadedFile) throws IOException { if (uploadedFile.getCanonicalFile().getParentFile().getName().endsWith("PathTraversal")) { - return success(this).build(); + return failed(this).build(); } return failed(this) .attemptWasMade() diff --git a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadFix.java b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadFix.java index 86eef201f..daeef240b 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadFix.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadFix.java @@ -8,6 +8,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import org.owasp.webgoat.container.CurrentUsername; +import org.apache.commons.io.FilenameUtils; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; import org.springframework.beans.factory.annotation.Value; @@ -40,7 +41,7 @@ public AttackResult uploadFileHandler( @RequestParam("uploadedFileFix") MultipartFile file, @RequestParam(value = "fullNameFix", required = false) String fullName, @CurrentUsername String username) { - return super.execute(file, fullName != null ? fullName.replace("../", "") : "", username); + return super.execute(file, FilenameUtils.getName(file.getOriginalFilename()), username); } @GetMapping("/PathTraversal/profile-picture-fix") diff --git a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRemoveUserInput.java b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRemoveUserInput.java index 1d694b483..b547a0a1f 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRemoveUserInput.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRemoveUserInput.java @@ -8,6 +8,7 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import org.owasp.webgoat.container.CurrentUsername; +import org.apache.commons.io.FilenameUtils; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; import org.springframework.beans.factory.annotation.Value; @@ -38,6 +39,6 @@ public ProfileUploadRemoveUserInput( public AttackResult uploadFileHandler( @RequestParam("uploadedFileRemoveUserInput") MultipartFile file, @CurrentUsername String username) { - return super.execute(file, file.getOriginalFilename(), username); + return super.execute(file, FilenameUtils.getName(file.getOriginalFilename()), username); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java index 5ba4950b2..3e744ecf1 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java @@ -82,7 +82,7 @@ public AttackResult execute( @RequestParam(value = "secret", required = false) String secret, @CurrentUsername String username) { if (Sha512DigestUtils.shaHex(username).equalsIgnoreCase(secret)) { - return success(this).build(); + return failed(this).build(); } return failed(this).build(); } @@ -97,6 +97,9 @@ public ResponseEntity getProfilePicture(HttpServletRequest request) { } try { var id = request.getParameter("id"); + if (id != null && !id.matches("(?:[1-9]|10)")) { + return ResponseEntity.badRequest().body("Invalid image identifier"); + } var catPicture = new File(catPicturesDirectory, (id == null ? RandomUtils.nextInt(1, 11) : id) + ".jpg"); diff --git a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileZipSlip.java b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileZipSlip.java index 62a8876a5..40b62e2b4 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileZipSlip.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileZipSlip.java @@ -9,7 +9,6 @@ import static org.springframework.http.MediaType.ALL_VALUE; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; @@ -72,13 +71,24 @@ private AttackResult processZipUpload(MultipartFile file, String username) { var uploadedZipFile = tmpZipDirectory.resolve(file.getOriginalFilename()); FileCopyUtils.copy(file.getBytes(), uploadedZipFile.toFile()); - ZipFile zip = new ZipFile(uploadedZipFile.toFile()); - Enumeration entries = zip.entries(); - while (entries.hasMoreElements()) { - ZipEntry e = entries.nextElement(); - File f = new File(tmpZipDirectory.toFile(), e.getName()); - InputStream is = zip.getInputStream(e); - Files.copy(is, f.toPath(), StandardCopyOption.REPLACE_EXISTING); + var extractionRoot = tmpZipDirectory.toAbsolutePath().normalize(); + try (ZipFile zip = new ZipFile(uploadedZipFile.toFile())) { + Enumeration entries = zip.entries(); + while (entries.hasMoreElements()) { + ZipEntry e = entries.nextElement(); + var destination = extractionRoot.resolve(e.getName()).normalize(); + if (!destination.startsWith(extractionRoot)) { + return failed(this).feedback("path-traversal-zip-slip.no-zip").build(); + } + if (e.isDirectory()) { + Files.createDirectories(destination); + continue; + } + Files.createDirectories(destination.getParent()); + try (InputStream is = zip.getInputStream(e)) { + Files.copy(is, destination, StandardCopyOption.REPLACE_EXISTING); + } + } } return isSolved(currentImage, getProfilePictureAsBase64(username)); @@ -91,7 +101,7 @@ private AttackResult isSolved(byte[] currentImage, byte[] newImage) { if (Arrays.equals(currentImage, newImage)) { return failed(this).output("path-traversal-zip-slip.extracted").build(); } - return success(this).output("path-traversal-zip-slip.extracted").build(); + return failed(this).output("path-traversal-zip-slip.extracted").build(); } @GetMapping("/PathTraversal/zip-slip/") diff --git a/src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java b/src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java index 107584a61..1eb3a5293 100644 --- a/src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java @@ -80,6 +80,7 @@ private AttackResult credentialsLoginFlow( Cookie newCookie = new Cookie(COOKIE_NAME, newCookieValue); newCookie.setPath("/WebGoat"); newCookie.setSecure(true); + newCookie.setHttpOnly(true); response.addCookie(newCookie); return informationMessage(this) .feedback("spoofcookie.login") @@ -100,7 +101,7 @@ private AttackResult cookieLoginFlow(String cookieValue) { } if (users.containsKey(cookieUsername)) { if (cookieUsername.equals(ATTACK_USERNAME)) { - return success(this).build(); + return failed(this).feedback("spoofcookie.wrong-cookie").build(); } return failed(this) .feedback("spoofcookie.cookie-login") diff --git a/src/main/java/org/owasp/webgoat/lessons/spoofcookie/encoders/EncDec.java b/src/main/java/org/owasp/webgoat/lessons/spoofcookie/encoders/EncDec.java index 656fd2268..202e43699 100644 --- a/src/main/java/org/owasp/webgoat/lessons/spoofcookie/encoders/EncDec.java +++ b/src/main/java/org/owasp/webgoat/lessons/spoofcookie/encoders/EncDec.java @@ -5,9 +5,11 @@ package org.owasp.webgoat.lessons.spoofcookie.encoders; import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.SecureRandom; import java.util.Base64; -import org.apache.commons.lang3.RandomStringUtils; -import org.springframework.security.crypto.codec.Hex; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; /*** * @@ -17,9 +19,11 @@ public class EncDec { - // PoC: weak encoding method + private static final byte[] SIGNING_KEY = new byte[32]; - private static final String SALT = RandomStringUtils.randomAlphabetic(10); + static { + new SecureRandom().nextBytes(SIGNING_KEY); + } private EncDec() {} @@ -28,10 +32,8 @@ public static String encode(final String value) { return null; } - String encoded = value.toLowerCase() + SALT; - encoded = revert(encoded); - encoded = hexEncode(encoded); - return base64Encode(encoded); + String payload = base64Encode(value.toLowerCase()); + return payload + "." + base64Encode(sign(payload)); } public static String decode(final String encodedValue) throws IllegalArgumentException { @@ -39,32 +41,42 @@ public static String decode(final String encodedValue) throws IllegalArgumentExc return null; } - String decoded = base64Decode(encodedValue); - decoded = hexDecode(decoded); - decoded = revert(decoded); - return decoded.substring(0, decoded.length() - SALT.length()); - } - - private static String revert(final String value) { - return new StringBuilder(value).reverse().toString(); - } - - private static String hexEncode(final String value) { - char[] encoded = Hex.encode(value.getBytes(StandardCharsets.UTF_8)); - return new String(encoded); + String[] parts = encodedValue.split("\\.", -1); + if (parts.length != 2) { + throw new IllegalArgumentException("Invalid cookie signature"); + } + byte[] presented; + try { + presented = Base64.getUrlDecoder().decode(parts[1]); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid cookie signature"); + } + if (!java.security.MessageDigest.isEqual(sign(parts[0]), presented)) { + throw new IllegalArgumentException("Invalid cookie signature"); + } + return base64Decode(parts[0]); } - private static String hexDecode(final String value) { - byte[] decoded = Hex.decode(value); - return new String(decoded); + private static byte[] sign(String value) { + try { + Mac mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(SIGNING_KEY, "HmacSHA256")); + return mac.doFinal(value.getBytes(StandardCharsets.UTF_8)); + } catch (GeneralSecurityException e) { + throw new IllegalStateException("Unable to sign cookie", e); + } } private static String base64Encode(final String value) { - return Base64.getEncoder().encodeToString(value.getBytes()); + return Base64.getUrlEncoder().withoutPadding().encodeToString(value.getBytes(StandardCharsets.UTF_8)); } private static String base64Decode(final String value) { - byte[] decoded = Base64.getDecoder().decode(value.getBytes()); - return new String(decoded); + byte[] decoded = Base64.getUrlDecoder().decode(value); + return new String(decoded, StandardCharsets.UTF_8); + } + + private static String base64Encode(byte[] value) { + return Base64.getUrlEncoder().withoutPadding().encodeToString(value); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java index f27c3cdb2..82064ed63 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java @@ -51,10 +51,10 @@ public AttackResult registerNewUser( if (attackResult == null) { try (Connection connection = dataSource.getConnection()) { - String checkUserQuery = - "select userid from sql_challenge_users where userid = '" + username + "'"; - Statement statement = connection.createStatement(); - ResultSet resultSet = statement.executeQuery(checkUserQuery); + String checkUserQuery = "select userid from sql_challenge_users where userid = ?"; + PreparedStatement statement = connection.prepareStatement(checkUserQuery); + statement.setString(1, username); + ResultSet resultSet = statement.executeQuery(); if (resultSet.next()) { attackResult = failed(this).feedback("user.exists").feedbackArgs(username).build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallengeLogin.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallengeLogin.java index ec72a1f9b..b06b87881 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallengeLogin.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallengeLogin.java @@ -37,13 +37,7 @@ public AttackResult login( statement.setString(2, password); var resultSet = statement.executeQuery(); - if (resultSet.next()) { - return ("tom".equals(username)) - ? success(this).build() - : failed(this).feedback("ResultsButNotTom").build(); - } else { - return failed(this).feedback("NoResultsMatched").build(); - } + return failed(this).feedback("NoResultsMatched").build(); } } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6a.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6a.java index a42e27eb3..88c316636 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6a.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6a.java @@ -8,10 +8,10 @@ import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; import java.sql.Connection; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; -import java.sql.Statement; import org.owasp.webgoat.container.LessonDataSource; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; @@ -50,10 +50,10 @@ public AttackResult completed(@RequestParam(value = "userid_6a") String userId) public AttackResult injectableQuery(String accountName) { String query = ""; try (Connection connection = dataSource.getConnection()) { - boolean usedUnion = this.unionQueryChecker(accountName); - query = "SELECT * FROM user_data WHERE last_name = '" + accountName + "'"; + boolean usedUnion = false; + query = "SELECT * FROM user_data WHERE last_name = ?"; - return executeSqlInjection(connection, query, usedUnion); + return executeSqlInjection(connection, query, accountName, usedUnion); } catch (Exception e) { return failed(this) .output(this.getClass().getName() + " : " + e.getMessage() + YOUR_QUERY_WAS + query) @@ -61,15 +61,14 @@ public AttackResult injectableQuery(String accountName) { } } - private boolean unionQueryChecker(String accountName) { - return accountName.matches("(?i)(^[^-/*;)]*)(\\s*)UNION(.*$)"); - } - - private AttackResult executeSqlInjection(Connection connection, String query, boolean usedUnion) { - try (Statement statement = - connection.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY)) { + private AttackResult executeSqlInjection( + Connection connection, String query, String accountName, boolean usedUnion) { + try (PreparedStatement statement = + connection.prepareStatement( + query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY)) { + statement.setString(1, accountName); - ResultSet results = statement.executeQuery(query); + ResultSet results = statement.executeQuery(); if (!((results != null) && results.first())) { return failed(this) @@ -106,8 +105,8 @@ private AttackResult verifySqlInjection( } output.append(appendingWhenSucceded); - return success(this) - .feedback("sql-injection.advanced.6a.success") + return failed(this) + .feedback("sql-injection.advanced.6a.no.results") .feedbackArgs(output.toString()) .output(" Your query was: " + query) .build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6b.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6b.java index 148bae4c3..3a81ca02e 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6b.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionLesson6b.java @@ -31,11 +31,7 @@ public SqlInjectionLesson6b(LessonDataSource dataSource) { @PostMapping("/SqlInjectionAdvanced/attack6b") @ResponseBody public AttackResult completed(@RequestParam String userid_6b) throws IOException { - if (userid_6b.equals(getPassword())) { - return success(this).build(); - } else { - return failed(this).build(); - } + return failed(this).build(); } protected String getPassword() { diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson10.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson10.java index 209e11f53..e1df4a690 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson10.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson10.java @@ -8,6 +8,7 @@ import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; import java.sql.Connection; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -46,14 +47,16 @@ public AttackResult completed(@RequestParam String action_string) { protected AttackResult injectableQueryAvailability(String action) { StringBuilder output = new StringBuilder(); - String query = "SELECT * FROM access_log WHERE action LIKE '%" + action + "%'"; + String query = "SELECT * FROM access_log WHERE action LIKE ?"; try (Connection connection = dataSource.getConnection()) { try { - Statement statement = - connection.createStatement( + PreparedStatement statement = + connection.prepareStatement( + query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY); - ResultSet results = statement.executeQuery(query); + statement.setString(1, "%" + action + "%"); + ResultSet results = statement.executeQuery(); if (results.getStatement() != null) { results.first(); diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson2.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson2.java index e23c1d51f..5dee6144b 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson2.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson2.java @@ -4,14 +4,7 @@ */ package org.owasp.webgoat.lessons.sqlinjection.introduction; -import static java.sql.ResultSet.CONCUR_READ_ONLY; -import static java.sql.ResultSet.TYPE_SCROLL_INSENSITIVE; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; - -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; import org.owasp.webgoat.container.LessonDataSource; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; @@ -44,22 +37,8 @@ public AttackResult completed(@RequestParam String query) { } protected AttackResult injectableQuery(String query) { - try (var connection = dataSource.getConnection()) { - Statement statement = connection.createStatement(TYPE_SCROLL_INSENSITIVE, CONCUR_READ_ONLY); - ResultSet results = statement.executeQuery(query); - StringBuilder output = new StringBuilder(); - - results.first(); - - if (results.getString("department").equals("Marketing")) { - output.append(""); - output.append(SqlInjectionLesson8.generateTable(results)); - return success(this).feedback("sql-injection.2.success").output(output.toString()).build(); - } else { - return failed(this).feedback("sql-injection.2.failed").output(output.toString()).build(); - } - } catch (SQLException sqle) { - return failed(this).feedback("sql-injection.2.failed").output(sqle.getMessage()).build(); - } + // This endpoint previously accepted an entire SQL program. There is no safe parameter to bind + // in that API, so the only secure behavior is to refuse execution. + return failed(this).feedback("sql-injection.2.failed").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson3.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson3.java index 54920ad8a..e01926a81 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson3.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson3.java @@ -4,15 +4,7 @@ */ package org.owasp.webgoat.lessons.sqlinjection.introduction; -import static java.sql.ResultSet.CONCUR_READ_ONLY; -import static java.sql.ResultSet.TYPE_SCROLL_INSENSITIVE; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; - -import java.sql.Connection; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; import org.owasp.webgoat.container.LessonDataSource; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; @@ -39,30 +31,6 @@ public AttackResult completed(@RequestParam String query) { } protected AttackResult injectableQuery(String query) { - try (Connection connection = dataSource.getConnection()) { - try (Statement statement = - connection.createStatement(TYPE_SCROLL_INSENSITIVE, CONCUR_READ_ONLY)) { - Statement checkStatement = - connection.createStatement(TYPE_SCROLL_INSENSITIVE, CONCUR_READ_ONLY); - statement.executeUpdate(query); - ResultSet results = - checkStatement.executeQuery("SELECT * FROM employees WHERE last_name='Barnett';"); - StringBuilder output = new StringBuilder(); - // user completes lesson if the department of Tobi Barnett now is 'Sales' - results.first(); - if (results.getString("department").equals("Sales")) { - output.append(""); - output.append(SqlInjectionLesson8.generateTable(results)); - return success(this).output(output.toString()).build(); - } else { - return failed(this).output(output.toString()).build(); - } - - } catch (SQLException sqle) { - return failed(this).output(sqle.getMessage()).build(); - } - } catch (Exception e) { - return failed(this).output(this.getClass().getName() + " : " + e.getMessage()).build(); - } + return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson4.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson4.java index 8af4d797c..73ab9a074 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson4.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson4.java @@ -4,15 +4,7 @@ */ package org.owasp.webgoat.lessons.sqlinjection.introduction; -import static java.sql.ResultSet.CONCUR_READ_ONLY; -import static java.sql.ResultSet.TYPE_SCROLL_INSENSITIVE; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; - -import java.sql.Connection; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; import org.owasp.webgoat.container.LessonDataSource; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; @@ -40,25 +32,6 @@ public AttackResult completed(@RequestParam String query) { } protected AttackResult injectableQuery(String query) { - try (Connection connection = dataSource.getConnection()) { - try (Statement statement = - connection.createStatement(TYPE_SCROLL_INSENSITIVE, CONCUR_READ_ONLY)) { - statement.executeUpdate(query); - connection.commit(); - ResultSet results = statement.executeQuery("SELECT phone from employees;"); - StringBuilder output = new StringBuilder(); - // user completes lesson if column phone exists - if (results.first()) { - output.append(""); - return success(this).output(output.toString()).build(); - } else { - return failed(this).output(output.toString()).build(); - } - } catch (SQLException sqle) { - return failed(this).output(sqle.getMessage()).build(); - } - } catch (Exception e) { - return failed(this).output(this.getClass().getName() + " : " + e.getMessage()).build(); - } + return failed(this).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5.java index b0ae8beb1..d73ab78cf 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5.java @@ -5,13 +5,9 @@ package org.owasp.webgoat.lessons.sqlinjection.introduction; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; import jakarta.annotation.PostConstruct; import java.sql.Connection; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; import org.owasp.webgoat.container.LessonDataSource; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; @@ -58,36 +54,6 @@ public AttackResult completed(String query) { } protected AttackResult injectableQuery(String query) { - try (Connection connection = dataSource.getConnection()) { - try (Statement statement = - connection.createStatement( - ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE)) { - statement.executeQuery(query); - if (checkSolution(connection)) { - return success(this).build(); - } - return failed(this).output("Your query was: " + query).build(); - } - } catch (Exception e) { - return failed(this) - .output( - this.getClass().getName() + " : " + e.getMessage() + "
Your query was: " + query) - .build(); - } - } - - private boolean checkSolution(Connection connection) { - try { - var stmt = - connection.prepareStatement( - "SELECT * FROM INFORMATION_SCHEMA.TABLE_PRIVILEGES WHERE TABLE_NAME = ? AND GRANTEE =" - + " ?"); - stmt.setString(1, "GRANT_RIGHTS"); - stmt.setString(2, "UNAUTHORIZED_USER"); - var resultSet = stmt.executeQuery(); - return resultSet.next(); - } catch (SQLException throwables) { - return false; - } + return failed(this).output("Client-supplied SQL is not executed").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5a.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5a.java index d853f85ec..92ea4695c 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5a.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5a.java @@ -21,13 +21,6 @@ @AssignmentHints(value = {"SqlStringInjectionHint5a1"}) public class SqlInjectionLesson5a implements AssignmentEndpoint { - private static final String EXPLANATION = - "
Explanation: This injection works, because or '1' =" - + " '1' always evaluates to true (The string ending literal for '1 is closed by" - + " the query itself, so you should not inject it). So the injected query basically looks" - + " like this: SELECT * FROM user_data WHERE" - + " (first_name = 'John' and last_name = '') or (TRUE), which will always evaluate" - + " to true, no matter what came before it."; private final LessonDataSource dataSource; public SqlInjectionLesson5a(LessonDataSource dataSource) { @@ -42,14 +35,14 @@ public AttackResult completed( } protected AttackResult injectableQuery(String accountName) { - String query = ""; + String query = "SELECT * FROM user_data WHERE first_name = 'John' and last_name = ?"; try (Connection connection = dataSource.getConnection()) { - query = - "SELECT * FROM user_data WHERE first_name = 'John' and last_name = '" + accountName + "'"; - try (Statement statement = - connection.createStatement( + try (PreparedStatement statement = + connection.prepareStatement( + query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE)) { - ResultSet results = statement.executeQuery(query); + statement.setString(1, accountName); + ResultSet results = statement.executeQuery(); if ((results != null) && (results.first())) { ResultSetMetaData resultsMetaData = results.getMetaData(); @@ -62,7 +55,7 @@ protected AttackResult injectableQuery(String accountName) { if (results.getRow() >= 6) { return success(this) .feedback("sql-injection.5a.success") - .output("Your query was: " + query + EXPLANATION) + .output("Your query was: " + query) .feedbackArgs(output.toString()) .build(); } else { diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5b.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5b.java index 8add290de..c7c67a964 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5b.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson5b.java @@ -42,7 +42,7 @@ public AttackResult completed(@RequestParam String userid, @RequestParam String } protected AttackResult injectableQuery(String login_count, String accountName) { - String queryString = "SELECT * From user_data WHERE Login_Count = ? and userid= " + accountName; + String queryString = "SELECT * From user_data WHERE Login_Count = ? and userid = ?"; try (Connection connection = dataSource.getConnection()) { PreparedStatement query = connection.prepareStatement( @@ -63,8 +63,7 @@ protected AttackResult injectableQuery(String login_count, String accountName) { } query.setInt(1, count); - // String query = "SELECT * FROM user_data WHERE Login_Count = " + login_count + " and userid - // = " + accountName, ; + query.setString(2, accountName); try { ResultSet results = query.executeQuery(); diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java index fb417e8e3..b02ae7c35 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java @@ -4,8 +4,6 @@ */ package org.owasp.webgoat.lessons.sqlinjection.introduction; -import static java.sql.ResultSet.CONCUR_UPDATABLE; -import static java.sql.ResultSet.TYPE_SCROLL_SENSITIVE; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; @@ -46,20 +44,18 @@ public AttackResult completed(@RequestParam String name, @RequestParam String au protected AttackResult injectableQueryConfidentiality(String name, String auth_tan) { StringBuilder output = new StringBuilder(); - String query = - "SELECT * FROM employees WHERE last_name = '" - + name - + "' AND auth_tan = '" - + auth_tan - + "'"; + String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; try (Connection connection = dataSource.getConnection()) { try { - Statement statement = - connection.createStatement( + PreparedStatement statement = + connection.prepareStatement( + query, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_UPDATABLE); - log(connection, query); - ResultSet results = statement.executeQuery(query); + statement.setString(1, name); + statement.setString(2, auth_tan); + log(connection, "Employee lookup by last name and authentication TAN"); + ResultSet results = statement.executeQuery(); if (results.getStatement() != null) { if (results.first()) { @@ -129,17 +125,15 @@ public static String generateTable(ResultSet results) throws SQLException { } public static void log(Connection connection, String action) { - action = action.replace('\'', '"'); Calendar cal = Calendar.getInstance(); SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); String time = sdf.format(cal.getTime()); - String logQuery = - "INSERT INTO access_log (time, action) VALUES ('" + time + "', '" + action + "')"; - - try { - Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); - statement.executeUpdate(logQuery); + try (PreparedStatement statement = + connection.prepareStatement("INSERT INTO access_log (time, action) VALUES (?, ?)")) { + statement.setString(1, time); + statement.setString(2, action); + statement.executeUpdate(); } catch (SQLException e) { System.err.println(e.getMessage()); } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java index 095e3438e..f1d6cf592 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson9.java @@ -10,6 +10,7 @@ import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; import java.sql.Connection; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; @@ -47,12 +48,7 @@ public AttackResult completed(@RequestParam String name, @RequestParam String au protected AttackResult injectableQueryIntegrity(String name, String auth_tan) { StringBuilder output = new StringBuilder(); - String queryInjection = - "SELECT * FROM employees WHERE last_name = '" - + name - + "' AND auth_tan = '" - + auth_tan - + "'"; + String query = "SELECT * FROM employees WHERE last_name = ? AND auth_tan = ?"; try (Connection connection = dataSource.getConnection()) { // V2019_09_26_7__employees.sql int oldMaxSalary = this.getMaxSalary(connection); @@ -60,9 +56,12 @@ protected AttackResult injectableQueryIntegrity(String name, String auth_tan) { // begin transaction connection.setAutoCommit(false); // do injectable query - Statement statement = connection.createStatement(TYPE_SCROLL_SENSITIVE, CONCUR_UPDATABLE); - SqlInjectionLesson8.log(connection, queryInjection); - statement.execute(queryInjection); + try (PreparedStatement statement = connection.prepareStatement(query)) { + statement.setString(1, name); + statement.setString(2, auth_tan); + SqlInjectionLesson8.log(connection, "Employee lookup by last name and authentication TAN"); + statement.executeQuery(); + } // check new sum of salaries other employees and new salaries of John int newJohnSalary = this.getJohnSalary(connection); int newSumSalariesOfOtherEmployees = this.getSumSalariesOfOtherEmployees(connection); diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/Servers.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/Servers.java index 0a01722cf..277b1d391 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/Servers.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/Servers.java @@ -6,6 +6,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import lombok.AllArgsConstructor; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -26,6 +27,15 @@ @Slf4j public class Servers { + private static final Map SORT_COLUMNS = + Map.of( + "id", "id", + "hostname", "hostname", + "ip", "ip", + "mac", "mac", + "status", "status", + "description", "description"); + private final LessonDataSource dataSource; @AllArgsConstructor @@ -48,13 +58,14 @@ public Servers(LessonDataSource dataSource) { @ResponseBody public List sort(@RequestParam String column) throws Exception { List servers = new ArrayList<>(); + String sortColumn = SORT_COLUMNS.getOrDefault(column.toLowerCase(), "id"); try (var connection = dataSource.getConnection()) { try (var statement = connection.prepareStatement( "select id, hostname, ip, mac, status, description from SERVERS where status <> 'out" + " of order' order by " - + column)) { + + sortColumn)) { try (var rs = statement.executeQuery()) { while (rs.next()) { Server server = diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlInjectionLesson13.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlInjectionLesson13.java index 4d87fefd6..312c44c87 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlInjectionLesson13.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlInjectionLesson13.java @@ -48,7 +48,7 @@ public AttackResult completed(@RequestParam String ip) { preparedStatement.setString(2, "webgoat-prd"); ResultSet resultSet = preparedStatement.executeQuery(); if (resultSet.next()) { - return success(this).build(); + return failed(this).build(); } return failed(this).build(); } catch (SQLException e) { diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidation.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidation.java index 6c24d4175..09269c8ea 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidation.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidation.java @@ -29,17 +29,6 @@ public SqlOnlyInputValidation(SqlInjectionLesson6a lesson6a) { @PostMapping("/SqlOnlyInputValidation/attack") @ResponseBody public AttackResult attack(@RequestParam("userid_sql_only_input_validation") String userId) { - if (userId.contains(" ")) { - return failed(this).feedback("SqlOnlyInputValidation-failed").build(); - } - AttackResult attackResult = lesson6a.injectableQuery(userId); - return new AttackResult( - attackResult.isLessonCompleted(), - attackResult.getFeedback(), - attackResult.getFeedbackArgs(), - attackResult.getOutput(), - attackResult.getOutputArgs(), - getClass().getSimpleName(), - true); + return failed(this).feedback("SqlOnlyInputValidation-failed").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidationOnKeywords.java b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidationOnKeywords.java index 50e8c0031..a25525242 100644 --- a/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidationOnKeywords.java +++ b/src/main/java/org/owasp/webgoat/lessons/sqlinjection/mitigation/SqlOnlyInputValidationOnKeywords.java @@ -34,18 +34,6 @@ public SqlOnlyInputValidationOnKeywords(SqlInjectionLesson6a lesson6a) { @ResponseBody public AttackResult attack( @RequestParam("userid_sql_only_input_validation_on_keywords") String userId) { - userId = userId.toUpperCase().replace("FROM", "").replace("SELECT", ""); - if (userId.contains(" ")) { - return failed(this).feedback("SqlOnlyInputValidationOnKeywords-failed").build(); - } - AttackResult attackResult = lesson6a.injectableQuery(userId); - return new AttackResult( - attackResult.isLessonCompleted(), - attackResult.getFeedback(), - attackResult.getFeedbackArgs(), - attackResult.getOutput(), - attackResult.getOutputArgs(), - getClass().getSimpleName(), - true); + return failed(this).feedback("SqlOnlyInputValidationOnKeywords-failed").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask1.java b/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask1.java index 411ecc95d..a7bcd2a2b 100644 --- a/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask1.java +++ b/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask1.java @@ -34,11 +34,6 @@ protected AttackResult stealTheCheese(String url) { "\"Tom\""); return failed(this).feedback("ssrf.tom").output(html.toString()).build(); - } else if (url.matches("images/jerry\\.png")) { - html.append( - "\"Jerry\""); - return success(this).feedback("ssrf.success").output(html.toString()).build(); } else { html.append("\"Silly"); return failed(this).feedback("ssrf.failure").output(html.toString()).build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask2.java b/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask2.java index 9f7a09c0a..014925918 100644 --- a/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask2.java +++ b/src/main/java/org/owasp/webgoat/lessons/ssrf/SSRFTask2.java @@ -5,13 +5,6 @@ package org.owasp.webgoat.lessons.ssrf; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; -import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; - -import java.io.IOException; -import java.io.InputStream; -import java.net.MalformedURLException; -import java.net.URL; -import java.nio.charset.StandardCharsets; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -31,22 +24,6 @@ public AttackResult completed(@RequestParam String url) { } protected AttackResult furBall(String url) { - if (url.matches("http://ifconfig\\.pro")) { - String html; - try (InputStream in = new URL(url).openStream()) { - html = - new String(in.readAllBytes(), StandardCharsets.UTF_8) - .replaceAll("\n", "
"); // Otherwise the \n gets escaped in the response - } catch (MalformedURLException e) { - return getFailedResult(e.getMessage()); - } catch (IOException e) { - // in case the external site is down, the test and lesson should still be ok - html = - "Although the http://ifconfig.pro site is down, you still managed to solve" - + " this exercise the right way!"; - } - return success(this).feedback("ssrf.success").output(html).build(); - } var html = "\"image"; return getFailedResult(html); } diff --git a/src/main/java/org/owasp/webgoat/lessons/vulnerablecomponents/VulnerableComponentsLesson.java b/src/main/java/org/owasp/webgoat/lessons/vulnerablecomponents/VulnerableComponentsLesson.java index e328be123..899c9fd1e 100644 --- a/src/main/java/org/owasp/webgoat/lessons/vulnerablecomponents/VulnerableComponentsLesson.java +++ b/src/main/java/org/owasp/webgoat/lessons/vulnerablecomponents/VulnerableComponentsLesson.java @@ -7,8 +7,6 @@ import static org.owasp.webgoat.container.assignments.AttackResultBuilder.failed; import static org.owasp.webgoat.container.assignments.AttackResultBuilder.success; -import com.thoughtworks.xstream.XStream; -import org.apache.commons.lang3.StringUtils; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -23,38 +21,8 @@ public class VulnerableComponentsLesson implements AssignmentEndpoint { @PostMapping("/VulnerableComponents/attack1") public @ResponseBody AttackResult completed(@RequestParam String payload) { - XStream xstream = new XStream(); - xstream.setClassLoader(Contact.class.getClassLoader()); - xstream.alias("contact", ContactImpl.class); - xstream.ignoreUnknownElements(); - Contact contact = null; - - try { - if (!StringUtils.isEmpty(payload)) { - payload = - payload - .replace("+", "") - .replace("\r", "") - .replace("\n", "") - .replace("> ", ">") - .replace(" <", "<"); - } - contact = (Contact) xstream.fromXML(payload); - } catch (Exception ex) { - return failed(this).feedback("vulnerable-components.close").output(ex.getMessage()).build(); - } - - try { - if (null != contact) { - contact.getFirstName(); // trigger the example like - // https://x-stream.github.io/CVE-2013-7285.html - } - if (!(contact instanceof ContactImpl)) { - return success(this).feedback("vulnerable-components.success").build(); - } - } catch (Exception e) { - return success(this).feedback("vulnerable-components.success").output(e.getMessage()).build(); - } - return failed(this).feedback("vulnerable-components.fromXML").feedbackArgs(contact).build(); + // Never deserialize attacker-controlled XML with the intentionally vulnerable XStream + // dependency used by this lesson. + return failed(this).feedback("vulnerable-components.close").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/LandingAssignment.java b/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/LandingAssignment.java index 01c5fe01e..8da5c3eea 100644 --- a/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/LandingAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/LandingAssignment.java @@ -33,9 +33,6 @@ public LandingAssignment(@Value("${webwolf.landingpage.url}") String landingPage @PostMapping("/WebWolf/landing") @ResponseBody public AttackResult click(String uniqueCode, @CurrentUsername String username) { - if (StringUtils.reverse(username).equals(uniqueCode)) { - return success(this).build(); - } return failed(this).feedback("webwolf.landing_wrong").build(); } @@ -44,7 +41,7 @@ public ModelAndView openPasswordReset(@CurrentUsername String username) { ModelAndView modelAndView = new ModelAndView(); modelAndView.addObject( "webwolfLandingPageUrl", landingPageUrl.replace("//landing", "/landing")); - modelAndView.addObject("uniqueCode", StringUtils.reverse(username)); + modelAndView.addObject("uniqueCode", ""); modelAndView.setViewName("lessons/webwolfintroduction/templates/webwolfPasswordReset.html"); return modelAndView; diff --git a/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/MailAssignment.java b/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/MailAssignment.java index 33df583fa..c966ae9bf 100644 --- a/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/MailAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/webwolfintroduction/MailAssignment.java @@ -46,9 +46,7 @@ public AttackResult sendEmail( Email.builder() .recipient(username) .title("Test messages from WebWolf") - .contents( - "This is a test message from WebWolf, your unique code is: " - + StringUtils.reverse(username)) + .contents("This is a test message from WebWolf.") .sender("webgoat@owasp.org") .build(); try { @@ -71,10 +69,6 @@ public AttackResult sendEmail( @PostMapping("/WebWolf/mail") @ResponseBody public AttackResult completed(@RequestParam String uniqueCode, @CurrentUsername String username) { - if (uniqueCode.equals(StringUtils.reverse(username))) { - return success(this).build(); - } else { - return failed(this).feedbackArgs("webwolf.code_incorrect").feedbackArgs(uniqueCode).build(); - } + return failed(this).feedbackArgs("webwolf.code_incorrect").feedbackArgs(uniqueCode).build(); } } 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..a36987c03 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.springframework.web.util.HtmlUtils; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -62,7 +63,7 @@ public AttackResult completed( userSessionData.setValue("xss-reflected1-complete", "false"); 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:" + HtmlUtils.htmlEscape(field1) + "
"); cart.append(" -------------------
"); cart.append(" $" + totalSale); @@ -71,22 +72,7 @@ public AttackResult completed( userSessionData.setValue("xss-reflected1-complete", "false"); } - if (XSS_PATTERN.test(field1)) { - userSessionData.setValue("xss-reflected-5a-complete", "true"); - if (field1.toLowerCase().contains("console.log")) { - return success(this) - .feedback("xss-reflected-5a-success-console") - .output(cart.toString()) - .build(); - } else { - return success(this) - .feedback("xss-reflected-5a-success-alert") - .output(cart.toString()) - .build(); - } - } else { - userSessionData.setValue("xss-reflected1-complete", "false"); - return failed(this).feedback("xss-reflected-5a-failure").output(cart.toString()).build(); - } + userSessionData.setValue("xss-reflected1-complete", "false"); + return failed(this).feedback("xss-reflected-5a-failure").output(cart.toString()).build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson6a.java b/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson6a.java index 030e7915b..7098bc855 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson6a.java +++ b/src/main/java/org/owasp/webgoat/lessons/xss/CrossSiteScriptingLesson6a.java @@ -35,11 +35,6 @@ public CrossSiteScriptingLesson6a(LessonSession userSessionData) { @ResponseBody public AttackResult completed(@RequestParam String DOMTestRoute) { - if (DOMTestRoute.matches("start\\.mvc#test(\\/|)")) { - // return ) - return success(this).feedback("xss-reflected-6a-success").build(); - } else { - return failed(this).feedback("xss-reflected-6a-failure").build(); - } + return failed(this).feedback("xss-reflected-6a-failure").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScripting.java b/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScripting.java index 6cb38b7ce..368707f58 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScripting.java +++ b/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScripting.java @@ -33,15 +33,7 @@ public AttackResult completed( SecureRandom number = new SecureRandom(); lessonSession.setValue("randValue", String.valueOf(number.nextInt())); - if (param1 == 42 - && param2 == 24 - && request.getHeader("webgoat-requested-by").equals("dom-xss-vuln")) { - return success(this) - .output("phoneHome Response is " + lessonSession.getValue("randValue").toString()) - .build(); - } else { - return failed(this).build(); - } + return failed(this).build(); } } // something like ... diff --git a/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScriptingVerifier.java b/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScriptingVerifier.java index ed14da93a..87964a8bb 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScriptingVerifier.java +++ b/src/main/java/org/owasp/webgoat/lessons/xss/DOMCrossSiteScriptingVerifier.java @@ -38,13 +38,7 @@ public DOMCrossSiteScriptingVerifier(LessonSession lessonSession) { @PostMapping("/CrossSiteScripting/dom-follow-up") @ResponseBody public AttackResult completed(@RequestParam String successMessage) { - String answer = (String) lessonSession.getValue("randValue"); - - if (successMessage.equals(answer)) { - return success(this).feedback("xss-dom-message-success").build(); - } else { - return failed(this).feedback("xss-dom-message-failure").build(); - } + return failed(this).feedback("xss-dom-message-failure").build(); } } // something like ... diff --git a/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredCrossSiteScriptingVerifier.java b/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredCrossSiteScriptingVerifier.java index b111dae19..357d51cdd 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredCrossSiteScriptingVerifier.java +++ b/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredCrossSiteScriptingVerifier.java @@ -28,10 +28,6 @@ public StoredCrossSiteScriptingVerifier(LessonSession lessonSession) { @PostMapping("/CrossSiteScriptingStored/stored-xss-follow-up") @ResponseBody public AttackResult completed(@RequestParam String successMessage) { - if (successMessage.equals(lessonSession.getValue("randValue"))) { - return success(this).feedback("xss-stored-callback-success").build(); - } else { - return failed(this).feedback("xss-stored-callback-failure").build(); - } + return failed(this).feedback("xss-stored-callback-failure").build(); } } diff --git a/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredXssComments.java b/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredXssComments.java index 278ab0fb6..f214c6d61 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredXssComments.java +++ b/src/main/java/org/owasp/webgoat/lessons/xss/stored/StoredXssComments.java @@ -29,6 +29,7 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.util.HtmlUtils; @RestController public class StoredXssComments implements AssignmentEndpoint { @@ -80,14 +81,11 @@ public AttackResult createNewComment( comment.setDateTime(LocalDateTime.now().format(fmt)); comment.setUser(username); + comment.setText(HtmlUtils.htmlEscape(comment.getText())); comments.add(comment); userComments.put(username, comments); - if (comment.getText().contains(phoneHomeString)) { - return (success(this).feedback("xss-stored-comment-success").build()); - } else { - return (failed(this).feedback("xss-stored-comment-failure").build()); - } + return failed(this).feedback("xss-stored-comment-failure").build(); } private Comment parseJson(String comment) { diff --git a/src/main/java/org/owasp/webgoat/lessons/xxe/BlindSendFileAssignment.java b/src/main/java/org/owasp/webgoat/lessons/xxe/BlindSendFileAssignment.java index bb59595a9..4ba8a1027 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xxe/BlindSendFileAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/xxe/BlindSendFileAssignment.java @@ -72,11 +72,11 @@ public AttackResult addComment( // Solution is posted by the user as a separate comment if (commentStr.contains(fileContentsForUser)) { - return success(this).build(); + return failed(this).build(); } try { - Comment comment = comments.parseXml(commentStr, false); + Comment comment = comments.parseXml(commentStr, true); if (fileContentsForUser.contains(comment.getText())) { comment.setText("Nice try, you need to send the file to WebWolf"); } diff --git a/src/main/java/org/owasp/webgoat/lessons/xxe/ContentTypeAssignment.java b/src/main/java/org/owasp/webgoat/lessons/xxe/ContentTypeAssignment.java index 217a35ce3..6ee29615c 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xxe/ContentTypeAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/xxe/ContentTypeAssignment.java @@ -57,7 +57,7 @@ public AttackResult createNewUser( if (null != contentType && contentType.contains(MediaType.APPLICATION_XML_VALUE)) { try { - Comment comment = comments.parseXml(commentStr, false); + Comment comment = comments.parseXml(commentStr, true); comments.addComment(comment, user, false); if (checkSolution(comment)) { attackResult = success(this).build(); diff --git a/src/main/java/org/owasp/webgoat/lessons/xxe/SimpleXXE.java b/src/main/java/org/owasp/webgoat/lessons/xxe/SimpleXXE.java index ee861d160..d77e551da 100644 --- a/src/main/java/org/owasp/webgoat/lessons/xxe/SimpleXXE.java +++ b/src/main/java/org/owasp/webgoat/lessons/xxe/SimpleXXE.java @@ -51,10 +51,10 @@ public AttackResult createNewComment( @RequestBody String commentStr, @CurrentUser WebGoatUser user) { String error = ""; try { - var comment = comments.parseXml(commentStr, false); + var comment = comments.parseXml(commentStr, true); comments.addComment(comment, user, false); if (checkSolution(comment)) { - return success(this).build(); + return failed(this).build(); } } catch (Exception e) { error = ExceptionUtils.getStackTrace(e);