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 62d32754693..d37c33f6b73 100644 --- a/src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java @@ -10,8 +10,13 @@ import jakarta.servlet.http.HttpServletRequest; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.KeySpec; +import java.util.Base64; import java.util.Random; -import javax.xml.bind.DatatypeConverter; +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; import org.owasp.webgoat.container.assignments.AssignmentEndpoint; import org.owasp.webgoat.container.assignments.AssignmentHints; import org.owasp.webgoat.container.assignments.AttackResult; @@ -25,65 +30,142 @@ @RestController @AssignmentHints({"crypto-hashing.hints.1", "crypto-hashing.hints.2"}) public class HashingAssignment implements AssignmentEndpoint { - public static final String[] SECRETS = {"secret", "admin", "password", "123456", "passw0rd"}; + // Secure password hashing parameters as per OWASP recommendations + private static final int PBKDF2_ITERATIONS = 310000; // High iteration count for PBKDF2 + private static final int HASH_KEY_LENGTH = 256; // 256-bit key length + private static final int SALT_LENGTH = 16; // 16 bytes = 128 bits of salt + private static final String SECURE_HASH_ALGORITHM = "PBKDF2WithHmacSHA256"; + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + + // Example secrets for the assignment - in real apps, never store passwords in code + private static final String[] SECRETS = {"secret", "admin", "password", "123456", "passw0rd"}; - @RequestMapping(path = "/crypto/hashing/md5", produces = MediaType.TEXT_HTML_VALUE) - @ResponseBody - public String getMd5(HttpServletRequest request) throws NoSuchAlgorithmException { + /** + * Securely hashes a password using PBKDF2 with SHA-256. + * + * @param password The password to hash + * @return A string in the format "iterations:base64(salt):base64(hash)" + * @throws NoSuchAlgorithmException If the PBKDF2 algorithm is not available + * @throws InvalidKeySpecException If there's an issue with the key specification + * @throws IllegalArgumentException If the password is null or empty + */ + private String hashPassword(String password) throws NoSuchAlgorithmException, InvalidKeySpecException { + if (password == null || password.isEmpty()) { + throw new IllegalArgumentException("Password cannot be null or empty"); + } - String md5Hash = (String) request.getSession().getAttribute("md5Hash"); - if (md5Hash == null) { + byte[] salt = new byte[SALT_LENGTH]; + SECURE_RANDOM.nextBytes(salt); + + KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, PBKDF2_ITERATIONS, HASH_KEY_LENGTH); + SecretKeyFactory factory = SecretKeyFactory.getInstance(SECURE_HASH_ALGORITHM); + + byte[] hash = factory.generateSecret(spec).getEncoded(); + return PBKDF2_ITERATIONS + ":" + Base64.getEncoder().encodeToString(salt) + ":" + + Base64.getEncoder().encodeToString(hash); + } - String secret = SECRETS[new Random().nextInt(SECRETS.length)]; + /** + * Verifies a password against a stored hash using PBKDF2. + * + * @param password The password to verify + * @param storedHash The stored hash in the format "iterations:base64(salt):base64(hash)" + * @return true if the password matches, false otherwise + * @throws NoSuchAlgorithmException If the PBKDF2 algorithm is not available + * @throws InvalidKeySpecException If there's an issue with the key specification + * @throws IllegalArgumentException If any input is invalid + */ + private boolean verifyPassword(String password, String storedHash) + throws NoSuchAlgorithmException, InvalidKeySpecException { + if (password == null || password.isEmpty() || storedHash == null || storedHash.isEmpty()) { + throw new IllegalArgumentException("Password and stored hash cannot be null or empty"); + } - MessageDigest md = MessageDigest.getInstance("MD5"); - md.update(secret.getBytes()); - byte[] digest = md.digest(); - md5Hash = DatatypeConverter.printHexBinary(digest).toUpperCase(); - request.getSession().setAttribute("md5Hash", md5Hash); - request.getSession().setAttribute("md5Secret", secret); + String[] parts = storedHash.split(":"); + if (parts.length != 3) { + throw new IllegalArgumentException("Invalid stored hash format"); + } + + try { + int iterations = Integer.parseInt(parts[0]); + byte[] salt = Base64.getDecoder().decode(parts[1]); + byte[] hash = Base64.getDecoder().decode(parts[2]); + + KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, iterations, hash.length * 8); + SecretKeyFactory factory = SecretKeyFactory.getInstance(SECURE_HASH_ALGORITHM); + byte[] testHash = factory.generateSecret(spec).getEncoded(); + + return MessageDigest.isEqual(hash, testHash); // Constant-time comparison + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid hash format: " + e.getMessage()); + } + } + + /** + * Creates or retrieves a securely hashed password. + * This endpoint replaces the old MD5 hashing endpoint with secure PBKDF2 hashing. + */ + @RequestMapping(path = "/crypto/hashing/md5", produces = MediaType.TEXT_HTML_VALUE) + @ResponseBody + public String getSecureHash(HttpServletRequest request) { + try { + String hashedPassword = (String) request.getSession().getAttribute("hashedPassword"); + if (hashedPassword == null) { + // Use SecureRandom instead of Random for cryptographic operations + String secret = SECRETS[SECURE_RANDOM.nextInt(SECRETS.length)]; + hashedPassword = hashPassword(secret); + request.getSession().setAttribute("hashedPassword", hashedPassword); + request.getSession().setAttribute("passwordSecret", secret); + } + return hashedPassword; + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new RuntimeException("Error generating secure hash", e); } - return md5Hash; } + /** + * Returns the same secure hash as getSecureHash(). + * This maintains compatibility while using secure PBKDF2 hashing instead of SHA-256. + */ @RequestMapping(path = "/crypto/hashing/sha256", produces = MediaType.TEXT_HTML_VALUE) @ResponseBody - public String getSha256(HttpServletRequest request) throws NoSuchAlgorithmException { - - String sha256 = (String) request.getSession().getAttribute("sha256"); - if (sha256 == null) { - String secret = SECRETS[new Random().nextInt(SECRETS.length)]; - sha256 = getHash(secret, "SHA-256"); - request.getSession().setAttribute("sha256Hash", sha256); - request.getSession().setAttribute("sha256Secret", secret); - } - return sha256; + public String getSha256(HttpServletRequest request) { + return getSecureHash(request); // Reuse the same secure hashing mechanism } + /** + * Validates the user's password guesses against the stored secret. + */ @PostMapping("/crypto/hashing") @ResponseBody public AttackResult completed( HttpServletRequest request, - @RequestParam String answer_pwd1, - @RequestParam String answer_pwd2) { + @RequestParam(required = false) String answer_pwd1, + @RequestParam(required = false) String answer_pwd2) { - String md5Secret = (String) request.getSession().getAttribute("md5Secret"); - String sha256Secret = (String) request.getSession().getAttribute("sha256Secret"); + try { + // Input validation + if (answer_pwd1 == null || answer_pwd2 == null || answer_pwd1.isEmpty() || answer_pwd2.isEmpty()) { + return failed(this).feedback("crypto-hashing.empty").build(); + } - if (answer_pwd1 != null && answer_pwd2 != null) { - if (answer_pwd1.equals(md5Secret) && answer_pwd2.equals(sha256Secret)) { - return success(this).feedback("crypto-hashing.success").build(); - } else if (answer_pwd1.equals(md5Secret) || answer_pwd2.equals(sha256Secret)) { - return failed(this).feedback("crypto-hashing.oneok").build(); - } - } - return failed(this).feedback("crypto-hashing.empty").build(); - } + String storedHash = (String) request.getSession().getAttribute("hashedPassword"); + String originalSecret = (String) request.getSession().getAttribute("passwordSecret"); + + // Verify both answers match the original secret + // Note: In a real application, we would use verifyPassword() instead of direct string comparison + boolean answer1Correct = answer_pwd1.equals(originalSecret); + boolean answer2Correct = answer_pwd2.equals(originalSecret); - public static String getHash(String secret, String algorithm) throws NoSuchAlgorithmException { - MessageDigest md = MessageDigest.getInstance(algorithm); - md.update(secret.getBytes()); - byte[] digest = md.digest(); - return DatatypeConverter.printHexBinary(digest).toUpperCase(); + if (answer1Correct && answer2Correct) { + return success(this).feedback("crypto-hashing.success").build(); + } else if (answer1Correct || answer2Correct) { + return failed(this).feedback("crypto-hashing.oneok").build(); + } + + return failed(this).feedback("crypto-hashing.invalid").build(); + } catch (Exception e) { + return failed(this).feedback("crypto-hashing.error").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 d69e721b3de..6d5a464e5f1 100644 --- a/src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java +++ b/src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java @@ -112,11 +112,17 @@ 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.setSecure(true); // Ensures cookie is only sent over HTTPS + cookie.setHttpOnly(true); // Prevents JavaScript access to the cookie + cookie.setPath("/"); // Ensure cookie path is set response.addCookie(cookie); response.setStatus(HttpStatus.OK.value()); response.setContentType(MediaType.APPLICATION_JSON_VALUE); } else { Cookie cookie = new Cookie("access_token", ""); + cookie.setSecure(true); // Ensures cookie is only sent over HTTPS + cookie.setHttpOnly(true); // Prevents JavaScript access to the cookie + cookie.setPath("/"); // Ensure cookie path is set response.addCookie(cookie); response.setStatus(HttpStatus.UNAUTHORIZED.value()); response.setContentType(MediaType.APPLICATION_JSON_VALUE); 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 5ba4950b213..216dbe3a78d 100644 --- a/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java +++ b/src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java @@ -16,7 +16,12 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Base64; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomUtils; import org.owasp.webgoat.container.CurrentUsername; @@ -48,11 +53,17 @@ }) @Slf4j public class ProfileUploadRetrieval implements AssignmentEndpoint { - private final File catPicturesDirectory; + private final Path catPicturesDirectory; + private final Map fileIdMapping = new HashMap<>(); + private final String secretFileName = "path-traversal-secret.jpg"; public ProfileUploadRetrieval(@Value("${webgoat.server.directory}") String webGoatHomeDirectory) { - this.catPicturesDirectory = new File(webGoatHomeDirectory, "/PathTraversal/" + "/cats"); - this.catPicturesDirectory.mkdirs(); + this.catPicturesDirectory = Paths.get(webGoatHomeDirectory, "PathTraversal", "cats").toAbsolutePath().normalize(); + try { + Files.createDirectories(this.catPicturesDirectory); + } catch (IOException e) { + log.error("Failed to create cat pictures directory", e); + } } @PostConstruct @@ -61,18 +72,34 @@ public void initAssignment() { try (InputStream is = new ClassPathResource("lessons/pathtraversal/images/cats/" + i + ".jpg") .getInputStream()) { - FileCopyUtils.copy(is, new FileOutputStream(new File(catPicturesDirectory, i + ".jpg"))); + String uuid = UUID.randomUUID().toString(); + Path targetFile = catPicturesDirectory.resolve(uuid + ".jpg").normalize(); + // Ensure the target file is within the allowed directory + if (!targetFile.startsWith(catPicturesDirectory)) { + log.error("Attempted path traversal during init: {}", targetFile); + continue; + } + FileCopyUtils.copy(is, new FileOutputStream(targetFile.toFile())); + fileIdMapping.put(String.valueOf(i), uuid); } catch (Exception e) { - log.error("Unable to copy pictures" + e.getMessage()); + log.error("Unable to copy pictures", e); } } - var secretDirectory = this.catPicturesDirectory.getParentFile().getParentFile(); try { + Path secretDirectory = catPicturesDirectory.getParent().getParent(); + Path secretFile = secretDirectory.resolve(secretFileName).normalize(); + + // Security check: Ensure the secret file is created in the expected location + if (!secretFile.startsWith(secretDirectory)) { + log.error("Invalid secret file location detected during init"); + return; + } + Files.writeString( - secretDirectory.toPath().resolve("path-traversal-secret.jpg"), + secretFile, "You found it submit the SHA-512 hash of your username as answer"); } catch (IOException e) { - log.error("Unable to write secret in: {}", secretDirectory, e); + log.error("Unable to write secret file", e); } } @@ -90,30 +117,58 @@ public AttackResult execute( @GetMapping("/PathTraversal/random-picture") @ResponseBody public ResponseEntity getProfilePicture(HttpServletRequest request) { - var queryParams = request.getQueryString(); - if (queryParams != null && (queryParams.contains("..") || queryParams.contains("/"))) { - return ResponseEntity.badRequest() - .body("Illegal characters are not allowed in the query params"); - } try { var id = request.getParameter("id"); - var catPicture = - new File(catPicturesDirectory, (id == null ? RandomUtils.nextInt(1, 11) : id) + ".jpg"); + String fileId; + + if (id == null) { + // For random picture requests, use a number between 1-10 to get the UUID + fileId = fileIdMapping.get(String.valueOf(RandomUtils.nextInt(1, 11))); + } else { + // For specific requests, first check if it's a direct number mapping + fileId = fileIdMapping.get(id); + if (fileId == null) { + // If not found in mapping, treat the id as a UUID + fileId = id; + } + } - if (catPicture.getName().toLowerCase().contains("path-traversal-secret.jpg")) { - return ResponseEntity.ok() - .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) - .body(FileCopyUtils.copyToByteArray(catPicture)); + // Construct and validate the file path + Path requestedFile = catPicturesDirectory.resolve(fileId + ".jpg").normalize(); + + // Security check: Ensure the resolved path is within the allowed directory + if (!requestedFile.startsWith(catPicturesDirectory)) { + log.warn("Attempted path traversal attack detected: {}", id); + return ResponseEntity.notFound().build(); } - if (catPicture.exists()) { + + // Check if file exists and handle appropriately + if (Files.exists(requestedFile)) { return ResponseEntity.ok() .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) - .location(new URI("/PathTraversal/random-picture?id=" + catPicture.getName())) - .body(Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(catPicture))); + .body(FileCopyUtils.copyToByteArray(requestedFile.toFile())); } - return ResponseEntity.status(HttpStatus.NOT_FOUND) - .location(new URI("/PathTraversal/random-picture?id=" + catPicture.getName())) - .body( + + // Special case for the secret file - only return if explicitly requested and properly mapped + if (fileId != null && fileId.toLowerCase().contains(secretFileName.toLowerCase())) { + Path secretFile = catPicturesDirectory.getParent().getParent().resolve(secretFileName).normalize(); + // Additional security check for secret file + if (!secretFile.startsWith(catPicturesDirectory.getParent().getParent())) { + log.warn("Attempted path traversal to unauthorized location: {}", secretFile); + return ResponseEntity.notFound().build(); + } + if (Files.exists(secretFile)) { + return ResponseEntity.ok() + .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) + .body(FileCopyUtils.copyToByteArray(secretFile.toFile())); + } + } + + return ResponseEntity.notFound().build(); + } catch (Exception e) { + log.warn("Error processing picture request: {}", e.getMessage()); + return ResponseEntity.notFound().build(); + } StringUtils.arrayToCommaDelimitedString(catPicture.getParentFile().listFiles()) .getBytes()); } catch (IOException | URISyntaxException e) { 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 107584a61a3..3c983e6d763 100644 --- a/src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java +++ b/src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java @@ -63,6 +63,10 @@ public AttackResult login( public void cleanup(HttpServletResponse response) { Cookie cookie = new Cookie(COOKIE_NAME, ""); cookie.setMaxAge(0); + cookie.setPath("/WebGoat"); + cookie.setSecure(true); + cookie.setHttpOnly(true); + response.setHeader("Set-Cookie", response.getHeader("Set-Cookie") + "; SameSite=Strict"); response.addCookie(cookie); } @@ -80,6 +84,8 @@ private AttackResult credentialsLoginFlow( Cookie newCookie = new Cookie(COOKIE_NAME, newCookieValue); newCookie.setPath("/WebGoat"); newCookie.setSecure(true); + newCookie.setHttpOnly(true); + response.setHeader("Set-Cookie", response.getHeader("Set-Cookie") + "; SameSite=Strict"); response.addCookie(newCookie); return informationMessage(this) .feedback("spoofcookie.login")