Fix zip slip lesson.

The lesson did not work properly as the directory is reused across several path traversal lessons. First thing before uploading the zip file we now clean the directory.

The html had a reference to a location of the profile picture, this was part of a hint but this only causes confusion as this is not indicating to where you need to upload the picture with the Zip Slip vulnerability.

The assignment now contains a direct hint as where the image needs to be saved. The assignment is about creating a vulnerable zip file and NOT about guessing where the image should be saved inside WebGoat.

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

@@ -3,10 +3,14 @@ package org.owasp.webgoat.lessons.pathtraversal;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.util.Arrays;
 import java.util.Base64;
+import java.util.List;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.SneakyThrows;
+import org.apache.commons.io.FilenameUtils;
 import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
 import org.owasp.webgoat.container.assignments.AttackResult;
 import org.owasp.webgoat.container.session.WebSession;
@@ -32,14 +36,9 @@ public class ProfileUploadBase extends AssignmentEndpoint {
       return failed(this).feedback("path-traversal-profile-empty-name").build();
     }
 
-    var uploadDirectory =
-        new File(this.webGoatHomeDirectory, "/PathTraversal/" + webSession.getUserName());
-    if (uploadDirectory.exists()) {
-      FileSystemUtils.deleteRecursively(uploadDirectory);
-    }
+    File uploadDirectory = cleanupAndCreateDirectoryForUser();
 
     try {
-      uploadDirectory.mkdirs();
       var uploadedFile = new File(uploadDirectory, fullName);
       uploadedFile.createNewFile();
       FileCopyUtils.copy(file.getBytes(), uploadedFile);
@@ -57,6 +56,17 @@ public class ProfileUploadBase extends AssignmentEndpoint {
     }
   }
 
+  @SneakyThrows
+  protected File cleanupAndCreateDirectoryForUser() {
+    var uploadDirectory =
+        new File(this.webGoatHomeDirectory, "/PathTraversal/" + webSession.getUserName());
+    if (uploadDirectory.exists()) {
+      FileSystemUtils.deleteRecursively(uploadDirectory);
+    }
+    Files.createDirectories(uploadDirectory.toPath());
+    return uploadDirectory;
+  }
+
   private boolean attemptWasMade(File expectedUploadDirectory, File uploadedFile)
       throws IOException {
     return !expectedUploadDirectory
@@ -87,18 +97,25 @@ public class ProfileUploadBase extends AssignmentEndpoint {
     var profileDirectoryFiles = profilePictureDirectory.listFiles();
 
     if (profileDirectoryFiles != null && profileDirectoryFiles.length > 0) {
-      try (var inputStream = new FileInputStream(profileDirectoryFiles[0])) {
-        return Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream));
-      } catch (IOException e) {
-        return defaultImage();
-      }
+      return Arrays.stream(profileDirectoryFiles)
+          .filter(file -> FilenameUtils.isExtension(file.getName(), List.of("jpg", "png")))
+          .findFirst()
+          .map(
+              file -> {
+                try (var inputStream = new FileInputStream(profileDirectoryFiles[0])) {
+                  return Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream));
+                } catch (IOException e) {
+                  return defaultImage();
+                }
+              })
+          .orElse(defaultImage());
     } else {
       return defaultImage();
     }
   }
 
   @SneakyThrows
-  private byte[] defaultImage() {
+  protected byte[] defaultImage() {
     var inputStream = getClass().getResourceAsStream("/images/account.png");
     return Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream));
   }

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

@@ -13,13 +13,19 @@ import java.util.Enumeration;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 import lombok.SneakyThrows;
+import lombok.extern.slf4j.Slf4j;
 import org.owasp.webgoat.container.assignments.AssignmentHints;
 import org.owasp.webgoat.container.assignments.AttackResult;
 import org.owasp.webgoat.container.session.WebSession;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.ResponseEntity;
 import org.springframework.util.FileCopyUtils;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.multipart.MultipartFile;
 
 @RestController
@@ -29,6 +35,7 @@ import org.springframework.web.multipart.MultipartFile;
   "path-traversal-zip-slip.hint3",
   "path-traversal-zip-slip.hint4"
 })
+@Slf4j
 public class ProfileZipSlip extends ProfileUploadBase {
 
   public ProfileZipSlip(
@@ -52,12 +59,9 @@ public class ProfileZipSlip extends ProfileUploadBase {
   @SneakyThrows
   private AttackResult processZipUpload(MultipartFile file) {
     var tmpZipDirectory = Files.createTempDirectory(getWebSession().getUserName());
-    var uploadDirectory =
-        new File(getWebGoatHomeDirectory(), "/PathTraversal/" + getWebSession().getUserName());
+    cleanupAndCreateDirectoryForUser();
     var currentImage = getProfilePictureAsBase64();
 
-    Files.createDirectories(uploadDirectory.toPath());
-
     try {
       var uploadedZipFile = tmpZipDirectory.resolve(file.getOriginalFilename());
       FileCopyUtils.copy(file.getBytes(), uploadedZipFile.toFile());