Patchwork PR: Autofix TEST1

src/main/java/org/owasp/webgoat/lessons/cryptography/HashingAssignment.java

@@ -10,8 +10,12 @@
 import jakarta.servlet.http.HttpServletRequest;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.Random;
-import javax.xml.bind.DatatypeConverter;
+import java.security.SecureRandom;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Base64;
+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;
@@ -26,38 +30,86 @@
 @AssignmentHints({"crypto-hashing.hints.1", "crypto-hashing.hints.2"})
 public class HashingAssignment implements AssignmentEndpoint {
   public static final String[] SECRETS = {"secret", "admin", "password", "123456", "passw0rd"};
+  private static final int ITERATIONS = 10000;
+  private static final int KEY_LENGTH = 256;
+  private static final int SALT_LENGTH = 16;
+  private static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA256";
 
-  @RequestMapping(path = "/crypto/hashing/md5", produces = MediaType.TEXT_HTML_VALUE)
-  @ResponseBody
-  public String getMd5(HttpServletRequest request) throws NoSuchAlgorithmException {
+  private String generateSecureHash(String password) throws NoSuchAlgorithmException, InvalidKeySpecException {
+    SecureRandom random = new SecureRandom();
+    byte[] salt = new byte[SALT_LENGTH];
+    random.nextBytes(salt);
 
-    String md5Hash = (String) request.getSession().getAttribute("md5Hash");
-    if (md5Hash == null) {
+    byte[] hash = pbkdf2(password.toCharArray(), salt, ITERATIONS, KEY_LENGTH);
 
-      String secret = SECRETS[new Random().nextInt(SECRETS.length)];
+    // Format: iterations:base64(salt):base64(hash)
+    return ITERATIONS + ":" + Base64.getEncoder().encodeToString(salt) + ":" + Base64.getEncoder().encodeToString(hash);
+  }
 
-      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);
+  private byte[] pbkdf2(char[] password, byte[] salt, int iterations, int keyLength) throws NoSuchAlgorithmException, InvalidKeySpecException {
+    PBEKeySpec spec = new PBEKeySpec(password, salt, iterations, keyLength);
+    try {
+      SecretKeyFactory skf = SecretKeyFactory.getInstance(PBKDF2_ALGORITHM);
+      return skf.generateSecret(spec).getEncoded();
+    } finally {
+      spec.clearPassword();
     }
-    return md5Hash;
   }
 
-  @RequestMapping(path = "/crypto/hashing/sha256", produces = MediaType.TEXT_HTML_VALUE)
+  private boolean verifyPassword(String password, String storedHash) throws NoSuchAlgorithmException, InvalidKeySpecException {
+    String[] parts = storedHash.split(":");
+    int iterations = Integer.parseInt(parts[0]);
+    byte[] salt = Base64.getDecoder().decode(parts[1]);
+    byte[] hash = Base64.getDecoder().decode(parts[2]);
+
+    char[] passwordChars = password.toCharArray();
+    try {
+      byte[] testHash = pbkdf2(passwordChars, salt, iterations, hash.length * 8);
+      return MessageDigest.isEqual(hash, testHash);
+    } finally {
+      // Clear sensitive data
+      for(int i = 0; i < passwordChars.length; i++) {
+        passwordChars[i] = 0;
+      }
+    }
+  }
+
+  @RequestMapping(path = "/crypto/hashing/md5", produces = MediaType.TEXT_HTML_VALUE)
   @ResponseBody
-  public String getSha256(HttpServletRequest request) throws NoSuchAlgorithmException {
+  public String getMd5(HttpServletRequest request) {
+    try {
+      String md5Hash = (String) request.getSession().getAttribute("md5Hash");
+      if (md5Hash == null) {
+        String secret = SECRETS[new SecureRandom().nextInt(SECRETS.length)];
+
+        // Generate secure hash instead of MD5
+        md5Hash = generateSecureHash(secret);
+        request.getSession().setAttribute("md5Hash", md5Hash);
+        request.getSession().setAttribute("md5Secret", secret);
+      }
+      return md5Hash;
+    } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
+      throw new RuntimeException("Error generating secure hash", e);
+    }
+  }
 
-    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);
+  @RequestMapping(path = "/crypto/hashing/sha256", produces = MediaType.TEXT_HTML_VALUE)
+  @ResponseBody
+  public String getSha256(HttpServletRequest request) {
+    try {
+      String sha256 = (String) request.getSession().getAttribute("sha256Hash");
+      if (sha256 == null) {
+        String secret = SECRETS[new SecureRandom().nextInt(SECRETS.length)];
+
+        // Generate secure hash instead of SHA-256
+        sha256 = generateSecureHash(secret);
+        request.getSession().setAttribute("sha256Hash", sha256);
+        request.getSession().setAttribute("sha256Secret", secret);
+      }
+      return sha256;
+    } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
+      throw new RuntimeException("Error generating secure hash", e);
     }
-    return sha256;
   }
 
   @PostMapping("/crypto/hashing")
@@ -66,24 +118,29 @@ public AttackResult completed(
       HttpServletRequest request,
       @RequestParam String answer_pwd1,
       @RequestParam String answer_pwd2) {
+    try {
+      String md5Secret = (String) request.getSession().getAttribute("md5Secret");
+      String sha256Secret = (String) request.getSession().getAttribute("sha256Secret");
+      String md5Hash = (String) request.getSession().getAttribute("md5Hash");
+      String sha256Hash = (String) request.getSession().getAttribute("sha256Hash");
 
-    String md5Secret = (String) request.getSession().getAttribute("md5Secret");
-    String sha256Secret = (String) request.getSession().getAttribute("sha256Secret");
-
-    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();
+      if (answer_pwd1 != null && answer_pwd2 != null) {
+        // For learning purposes, we still compare with the original secrets
+        // In a real application, we would only store and verify the hashes
+        if (answer_pwd1.equals(md5Secret) && answer_pwd2.equals(sha256Secret)) {
+          // Demonstrate that verification works with the secure hashes
+          boolean validHash1 = verifyPassword(answer_pwd1, md5Hash);
+          boolean validHash2 = verifyPassword(answer_pwd2, sha256Hash);
+          if (validHash1 && validHash2) {
+            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();
+    } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
+      throw new RuntimeException("Error verifying password", e);
     }
-    return failed(this).feedback("crypto-hashing.empty").build();
-  }
-
-  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();
   }
 }

src/main/java/org/owasp/webgoat/lessons/jwt/JWTVotesEndpoint.java

@@ -112,11 +112,19 @@ 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.setMaxAge((int)Duration.ofDays(10).toSeconds());  // Match JWT expiration
+      cookie.setPath("/");           // Restrict cookie to root path
       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.setMaxAge(0);           // Immediately expire the cookie
+      cookie.setPath("/");           // Restrict cookie to root path
       response.addCookie(cookie);
       response.setStatus(HttpStatus.UNAUTHORIZED.value());
       response.setContentType(MediaType.APPLICATION_JSON_VALUE);

src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadRetrieval.java

@@ -90,36 +90,57 @@ 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");
-
-      if (catPicture.getName().toLowerCase().contains("path-traversal-secret.jpg")) {
-        return ResponseEntity.ok()
-            .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE))
-            .body(FileCopyUtils.copyToByteArray(catPicture));
+      if (id == null) {
+        id = String.valueOf(RandomUtils.nextInt(1, 11));
+      }
+
+      // Validate the id parameter (only allow alphanumeric and limited special chars)
+      if (!id.matches("^[a-zA-Z0-9-_]+$")) {
+        log.warn("Invalid characters in file name request: {}", id);
+        return ResponseEntity.badRequest().body("Invalid file name");
       }
+
+      // Add input length validation
+      if (id.length() > 50) {
+        log.warn("File name too long: {}", id);
+        return ResponseEntity.badRequest().body("Invalid file name");
+      }
+
+      java.nio.file.Path basePath = catPicturesDirectory.toPath().normalize();
+      java.nio.file.Path requestedPath = basePath.resolve(id + ".jpg").normalize();
+
+      // Verify the resolved path is within the allowed directory
+      if (!requestedPath.startsWith(basePath)) {
+        log.warn("Path traversal attempt detected: {}", requestedPath);
+        return ResponseEntity.badRequest().body("Invalid file path");
+      }
+
+      File catPicture = requestedPath.toFile();
+
+      // Verify file exists and check MIME type
       if (catPicture.exists()) {
+        String mimeType = Files.probeContentType(requestedPath);
+        if (mimeType == null || !mimeType.equals(MediaType.IMAGE_JPEG_VALUE)) {
+          log.warn("Invalid file type detected for file: {}", id);
+          return ResponseEntity.badRequest().body("Invalid file type");
+        }
+
         return ResponseEntity.ok()
-            .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE))
-            .location(new URI("/PathTraversal/random-picture?id=" + catPicture.getName()))
+            .contentType(MediaType.IMAGE_JPEG)
+            .location(new URI("/PathTraversal/random-picture?id=" + id))
             .body(Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(catPicture)));
       }
-      return ResponseEntity.status(HttpStatus.NOT_FOUND)
-          .location(new URI("/PathTraversal/random-picture?id=" + catPicture.getName()))
-          .body(
-              StringUtils.arrayToCommaDelimitedString(catPicture.getParentFile().listFiles())
-                  .getBytes());
+
+      // Return generic 404 without revealing directory contents
+      return ResponseEntity.notFound().build();
+
     } catch (IOException | URISyntaxException e) {
-      log.error("Image not found", e);
+      // Log the error but return generic message
+      log.error("Error processing file request for id: {}", request.getParameter("id"), e);
+      return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+          .body("An error occurred processing your request");
     }
-
-    return ResponseEntity.badRequest().build();
   }
 }

src/main/java/org/owasp/webgoat/lessons/spoofcookie/SpoofCookieAssignment.java

@@ -62,7 +62,12 @@ public AttackResult login(
   @GetMapping(path = "/SpoofCookie/cleanup")
   public void cleanup(HttpServletResponse response) {
     Cookie cookie = new Cookie(COOKIE_NAME, "");
+    cookie.setPath("/WebGoat");
     cookie.setMaxAge(0);
+    cookie.setSecure(true);
+    cookie.setHttpOnly(true);
+    // Set SameSite attribute through header
+    response.setHeader("Set-Cookie", String.format("%s=; Path=/WebGoat; Secure; HttpOnly; SameSite=Strict; Max-Age=0", COOKIE_NAME));
     response.addCookie(cookie);
   }
 
@@ -80,6 +85,9 @@ private AttackResult credentialsLoginFlow(
       Cookie newCookie = new Cookie(COOKIE_NAME, newCookieValue);
       newCookie.setPath("/WebGoat");
       newCookie.setSecure(true);
+      newCookie.setHttpOnly(true);
+      // Set SameSite attribute through header
+      response.setHeader("Set-Cookie", String.format("%s=%s; Path=/WebGoat; Secure; HttpOnly; SameSite=Strict", COOKIE_NAME, newCookieValue));
       response.addCookie(newCookie);
       return informationMessage(this)
           .feedback("spoofcookie.login")