From b3840e60e34f78663d15f12b3a483ab73349ad94 Mon Sep 17 00:00:00 2001 From: Nanne Baars Date: Sun, 8 Mar 2020 14:18:38 +0100 Subject: [PATCH] Fix lessons --- .../org/owasp/webgoat/PathTraversalTest.java | 16 ++----- .../path_traversal/ProfileUploadBase.java | 11 ++--- .../path_traversal/ProfileUploadFix.java | 2 +- .../ProfileUploadRemoveUserInput.java | 2 +- .../ProfileUploadRetrieval.java | 7 +-- .../main/resources/html/PathTraversal.html | 18 ++++---- .../resources/i18n/WebGoatLabels.properties | 1 + .../src/main/resources/images/cats/5.jpg | Bin 55654 -> 45502 bytes .../src/main/resources/js/path_traversal.js | 4 +- .../lessonPlans/en/PathTraversal_intro.adoc | 15 +------ .../en/PathTraversal_retrieval.adoc | 2 +- .../en/PathTraversal_upload_fix.adoc | 2 +- ...athTraversal_upload_remove_user_input.adoc | 2 +- .../path_traversal/ProfileUploadBaseTest.java | 42 ++++++++++++++++++ .../path_traversal/ProfileUploadFixTest.java | 5 +++ .../path_traversal/ProfileUploadTest.java | 24 ++++++++++ 16 files changed, 101 insertions(+), 52 deletions(-) create mode 100644 webgoat-lessons/path-traversal/src/test/java/org/owasp/webgoat/path_traversal/ProfileUploadBaseTest.java diff --git a/webgoat-integration-tests/src/test/java/org/owasp/webgoat/PathTraversalTest.java b/webgoat-integration-tests/src/test/java/org/owasp/webgoat/PathTraversalTest.java index 728b35df4..22a0606f4 100644 --- a/webgoat-integration-tests/src/test/java/org/owasp/webgoat/PathTraversalTest.java +++ b/webgoat-integration-tests/src/test/java/org/owasp/webgoat/PathTraversalTest.java @@ -2,10 +2,7 @@ package org.owasp.webgoat; import io.restassured.RestAssured; import org.hamcrest.CoreMatchers; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; +import org.junit.*; import org.junit.rules.TemporaryFolder; import org.springframework.security.core.token.Sha512DigestUtils; @@ -16,15 +13,8 @@ import java.util.Map; public class PathTraversalTest extends IntegrationTest { - private static String OS = System.getProperty("os.name").toLowerCase(); @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); - private File folder; - - @Before - public void setup() throws IOException { - this.folder = temporaryFolder.newFolder(); - } @Test public void assignment1() throws IOException { @@ -75,7 +65,7 @@ public class PathTraversalTest extends IntegrationTest { .when() .relaxedHTTPSValidation() .cookie("JSESSIONID", getWebGoatCookie()) - .multiPart("uploadedFileRetrieval", "../test.jpg", Files.readAllBytes(fileToUpload.toPath())) + .multiPart("uploadedFileRemoveUserInput", "../test.jpg", Files.readAllBytes(fileToUpload.toPath())) .post("/WebGoat/PathTraversal/profile-upload-remove-user-input") .then() .statusCode(200) @@ -90,7 +80,7 @@ public class PathTraversalTest extends IntegrationTest { .when() .relaxedHTTPSValidation() .cookie("JSESSIONID", getWebGoatCookie()) - .get("/WebGoat/PathTraversal/random?id=../../path-traversal-secret") + .get("/WebGoat/PathTraversal/random-picture?id=../../path-traversal-secret") .then() .statusCode(200) .content(CoreMatchers.is("You found it submit the SHA-512 hash of your username as answer")); diff --git a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadBase.java b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadBase.java index 558d162b8..42a4bdf3f 100644 --- a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadBase.java +++ b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadBase.java @@ -5,15 +5,11 @@ import lombok.SneakyThrows; import org.owasp.webgoat.assignments.AssignmentEndpoint; import org.owasp.webgoat.assignments.AttackResult; import org.owasp.webgoat.session.WebSession; -import org.springframework.beans.factory.annotation.Value; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.util.FileCopyUtils; import org.springframework.util.FileSystemUtils; import org.springframework.util.StringUtils; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.RequestParam; -import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.multipart.MultipartFile; import java.io.File; @@ -45,7 +41,8 @@ public class ProfileUploadBase extends AssignmentEndpoint { var uploadedFile = new File(uploadDirectory, fullName); uploadedFile.createNewFile(); FileCopyUtils.copy(file.getBytes(), uploadedFile); - if (userAttemptedToSolveLesson(uploadDirectory, uploadedFile)) { + + if (attemptWasMade(uploadDirectory, uploadedFile)) { return solvedIt(uploadedFile); } return informationMessage(this).feedback("path-traversal-profile-updated").feedbackArgs(uploadedFile.getAbsoluteFile()).build(); @@ -55,7 +52,7 @@ public class ProfileUploadBase extends AssignmentEndpoint { } } - private boolean userAttemptedToSolveLesson(File expectedUploadDirectory, File uploadedFile) throws IOException { + private boolean attemptWasMade(File expectedUploadDirectory, File uploadedFile) throws IOException { return !expectedUploadDirectory.getCanonicalPath().equals(uploadedFile.getParentFile().getCanonicalPath()); } @@ -63,7 +60,7 @@ public class ProfileUploadBase extends AssignmentEndpoint { if (uploadedFile.getCanonicalFile().getParentFile().getName().endsWith("PathTraversal")) { return success(this).build(); } - return failed(this).build(); + return failed(this).attemptWasMade().feedback("path-traversal-profile-attempt").feedbackArgs(uploadedFile.getCanonicalPath()).build(); } public ResponseEntity getProfilePicture() { diff --git a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadFix.java b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadFix.java index e5543c9f5..ee9add857 100644 --- a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadFix.java +++ b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadFix.java @@ -24,7 +24,7 @@ public class ProfileUploadFix extends ProfileUploadBase { public AttackResult uploadFileHandler( @RequestParam("uploadedFileFix") MultipartFile file, @RequestParam(value = "fullNameFix", required = false) String fullName) { - return super.execute(file, fullName != null ? fullName.replaceAll("../", "") : ""); + return super.execute(file, fullName != null ? fullName.replace("../", "") : ""); } @GetMapping("/PathTraversal/profile-picture-fix") diff --git a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRemoveUserInput.java b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRemoveUserInput.java index 1c1679f32..9a892d6b5 100644 --- a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRemoveUserInput.java +++ b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRemoveUserInput.java @@ -23,7 +23,7 @@ public class ProfileUploadRemoveUserInput extends ProfileUploadBase { @PostMapping(value = "/PathTraversal/profile-upload-remove-user-input", consumes = ALL_VALUE, produces = APPLICATION_JSON_VALUE) @ResponseBody - public AttackResult uploadFileHandler(@RequestParam("uploadedFileRetrieval") MultipartFile file) { + public AttackResult uploadFileHandler(@RequestParam("uploadedFileRemoveUserInput") MultipartFile file) { return super.execute(file, file.getOriginalFilename()); } } diff --git a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRetrieval.java b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRetrieval.java index 7a87963ea..ebdd69fd8 100644 --- a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRetrieval.java +++ b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileUploadRetrieval.java @@ -55,14 +55,15 @@ public class ProfileUploadRetrieval extends AssignmentEndpoint { } @PostMapping("/PathTraversal/random") - protected AttackResult execute(@RequestParam(value = "secret", required = false) String secret) { + @ResponseBody + public AttackResult execute(@RequestParam(value = "secret", required = false) String secret) { if (Sha512DigestUtils.shaHex(getWebSession().getUserName()).equalsIgnoreCase(secret)) { return success(this).build(); } return failed(this).build(); } - @GetMapping("/PathTraversal/random") + @GetMapping("/PathTraversal/random-picture") @ResponseBody public ResponseEntity getProfilePicture(@RequestParam(value = "id", required = false) String id) { var catPicture = new File(catPicturesDirectory, (id == null ? RandomUtils.nextInt(1, 11) : id) + ".jpg"); @@ -75,7 +76,7 @@ public class ProfileUploadRetrieval extends AssignmentEndpoint { } return ResponseEntity.ok() .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) - .location(new URI("/PathTraversal/random?id=" + catPicture.getName())) + .location(new URI("/PathTraversal/random-picture?id=" + catPicture.getName())) .body(Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(catPicture))); } catch (IOException | URISyntaxException e) { log.error("Unable to download picture", e); diff --git a/webgoat-lessons/path-traversal/src/main/resources/html/PathTraversal.html b/webgoat-lessons/path-traversal/src/main/resources/html/PathTraversal.html index 7e253645e..dc40205f7 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/html/PathTraversal.html +++ b/webgoat-lessons/path-traversal/src/main/resources/html/PathTraversal.html @@ -129,7 +129,7 @@ successCallback="profileUploadCallbackRemoveUserInput" failureCallback="profileUploadCallbackRemoveUserInput" informationalCallback="profileUploadCallbackRemoveUserInput" - prepareData="profileUploadFix" + prepareData="profileUploadRemoveUserInput" enctype="multipart/form-data" action="/WebGoat/PathTraversal/profile-upload-remove-user-input">
@@ -176,19 +176,21 @@
-
-
-
- + +
+
+
+
- +

-
+