diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 34aeb2ba7..7a68efd3c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,6 +11,7 @@ on: branches: - master - develop + - release/* tags-ignore: - '*' paths-ignore: diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties index 33a28a5a0..ffdc10e59 100644 --- a/.mvn/wrapper/maven-wrapper.properties +++ b/.mvn/wrapper/maven-wrapper.properties @@ -1,2 +1,2 @@ -distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.2.5/apache-maven-3.2.5-bin.zip -wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.5/maven-wrapper-0.5.5.jar +distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.1/apache-maven-3.8.1-bin.zip +wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e2b97458b..7734db368 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,5 +1,44 @@ # WebGoat release notes +## Version 8.2.0 + +### New functionality + +- Add new zip slip lesson (part of path traversal) +- SQL lessons are now separate for each user, database are now per user and no longer shared across users +- Moved to Java 15 & Spring Boot 2.4 & moved to JUnit 5 + +### Bug fixes + +- [#974 SQL injection Intro 5 not solvable](https://github.com/WebGoat/WebGoat/issues/974) +- [#962 SQL-Lesson 5 (Advanced) Solvable with wrong anwser](https://github.com/WebGoat/WebGoat/issues/962) +- [#961 SQl-Injection lesson 4 not deleting created row](https://github.com/WebGoat/WebGoat/issues/961) +- [#949 Challenge: Admin password reset always solvable](https://github.com/WebGoat/WebGoat/issues/949) +- [#923 - Upgrade to Java 15](https://github.com/WebGoat/WebGoat/issues/923) +- [#922 - Vulnerable components lesson](https://github.com/WebGoat/WebGoat/issues/922) +- [#891 - Update the OWASP website with the new all-in-one Docker container](https://github.com/WebGoat/WebGoat/issues/891) +- [#844 - Suggestion: Update navigation](https://github.com/WebGoat/WebGoat/issues/844) +- [#843 - Bypass front-end restrictions: Field restrictions - confusing text in form](https://github.com/WebGoat/WebGoat/issues/843) +- [#841 - XSS - Reflected XSS confusing instruction and success messages](https://github.com/WebGoat/WebGoat/issues/841) +- [#839 - SQL Injection (mitigation) Order by clause confusing](https://github.com/WebGoat/WebGoat/issues/839) +- [#838 - SQL mitigation (filtering) can only be passed by updating table](https://github.com/WebGoat/WebGoat/issues/838) + +## Contributors + +Special thanks to the following contributors providing us with a pull request: + +- nicholas-quirk +- VijoPlays +- aolle +- trollingHeifer +- maximmasiutin +- toshihue +- avivmu +- KellyMarchewa +- NatasG +- gabe-sky + + ## Version 8.1.0 ### New functionality diff --git a/docker/pom.xml b/docker/pom.xml index 70ee16e14..989ab28d3 100644 --- a/docker/pom.xml +++ b/docker/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat webgoat-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/pom.xml b/pom.xml index e3f19dddb..7680f8d1e 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat webgoat-parent pom - 8.2.0-SNAPSHOT + 8.2.0 WebGoat Parent Pom Parent Pom for the WebGoat Project. A deliberately insecure Web Application diff --git a/webgoat-container/pom.xml b/webgoat-container/pom.xml index 96d0ca51d..ca6e3e57e 100644 --- a/webgoat-container/pom.xml +++ b/webgoat-container/pom.xml @@ -9,7 +9,7 @@ org.owasp.webgoat webgoat-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/AsciiDoctorTemplateResolver.java b/webgoat-container/src/main/java/org/owasp/webgoat/AsciiDoctorTemplateResolver.java index f83d34ee5..e31524f09 100644 --- a/webgoat-container/src/main/java/org/owasp/webgoat/AsciiDoctorTemplateResolver.java +++ b/webgoat-container/src/main/java/org/owasp/webgoat/AsciiDoctorTemplateResolver.java @@ -84,6 +84,7 @@ public class AsciiDoctorTemplateResolver extends FileTemplateResolver { extensionRegistry.inlineMacro("webGoatVersion", WebGoatVersionMacro.class); extensionRegistry.inlineMacro("webGoatTempDir", WebGoatTmpDirMacro.class); extensionRegistry.inlineMacro("operatingSystem", OperatingSystemMacro.class); + extensionRegistry.inlineMacro("username", UsernameMacro.class); StringWriter writer = new StringWriter(); asciidoctor.convert(new InputStreamReader(is), writer, createAttributes()); diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/OperatingSystemMacro.java b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/OperatingSystemMacro.java index 91613c309..4671a9fd5 100644 --- a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/OperatingSystemMacro.java +++ b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/OperatingSystemMacro.java @@ -17,6 +17,9 @@ public class OperatingSystemMacro extends InlineMacroProcessor { @Override public Object process(ContentNode contentNode, String target, Map attributes) { - return System.getProperty("os.name"); + var osName = System.getProperty("os.name"); + + //see https://discuss.asciidoctor.org/How-to-create-inline-macro-producing-HTML-In-AsciidoctorJ-td8313.html for why quoted is used + return createPhraseNode(contentNode, "quoted", osName); } } diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/UsernameMacro.java b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/UsernameMacro.java new file mode 100644 index 000000000..f44e2cf62 --- /dev/null +++ b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/UsernameMacro.java @@ -0,0 +1,31 @@ +package org.owasp.webgoat.asciidoc; + +import org.asciidoctor.ast.ContentNode; +import org.asciidoctor.extension.InlineMacroProcessor; +import org.owasp.webgoat.users.WebGoatUser; +import org.springframework.security.core.context.SecurityContextHolder; + +import java.util.Map; + +public class UsernameMacro extends InlineMacroProcessor { + + public UsernameMacro(String macroName) { + super(macroName); + } + + public UsernameMacro(String macroName, Map config) { + super(macroName, config); + } + + @Override + public Object process(ContentNode contentNode, String target, Map attributes) { + var auth = SecurityContextHolder.getContext().getAuthentication(); + var username = "unknown"; + if (auth.getPrincipal() instanceof WebGoatUser) { + username = ((WebGoatUser) auth.getPrincipal()).getUsername(); + } + + //see https://discuss.asciidoctor.org/How-to-create-inline-macro-producing-HTML-In-AsciidoctorJ-td8313.html for why quoted is used + return createPhraseNode(contentNode, "quoted", username); + } +} diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatTmpDirMacro.java b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatTmpDirMacro.java index 3310a22fe..0636e9823 100644 --- a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatTmpDirMacro.java +++ b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatTmpDirMacro.java @@ -16,7 +16,11 @@ public class WebGoatTmpDirMacro extends InlineMacroProcessor { } @Override - public String process(ContentNode contentNode, String target, Map attributes) { - return EnvironmentExposure.getEnv().getProperty("webgoat.server.directory"); + public Object process(ContentNode contentNode, String target, Map attributes) { + var env = EnvironmentExposure.getEnv().getProperty("webgoat.server.directory"); + + //see https://discuss.asciidoctor.org/How-to-create-inline-macro-producing-HTML-In-AsciidoctorJ-td8313.html for why quoted is used + return createPhraseNode(contentNode, "quoted", env); + } } diff --git a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatVersionMacro.java b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatVersionMacro.java index 8029ab0fd..15ff78ae5 100644 --- a/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatVersionMacro.java +++ b/webgoat-container/src/main/java/org/owasp/webgoat/asciidoc/WebGoatVersionMacro.java @@ -16,7 +16,10 @@ public class WebGoatVersionMacro extends InlineMacroProcessor { } @Override - public String process(ContentNode contentNode, String target, Map attributes) { - return EnvironmentExposure.getEnv().getProperty("webgoat.build.version"); + public Object process(ContentNode contentNode, String target, Map attributes) { + var webgoatVersion = EnvironmentExposure.getEnv().getProperty("webgoat.build.version"); + + //see https://discuss.asciidoctor.org/How-to-create-inline-macro-producing-HTML-In-AsciidoctorJ-td8313.html for why quoted is used + return createPhraseNode(contentNode, "quoted", webgoatVersion); } } diff --git a/webgoat-integration-tests/pom.xml b/webgoat-integration-tests/pom.xml index c5b095eae..e96e4b67f 100644 --- a/webgoat-integration-tests/pom.xml +++ b/webgoat-integration-tests/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat webgoat-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-integration-tests/src/test/java/org/owasp/webgoat/DeserializationTest.java b/webgoat-integration-tests/src/test/java/org/owasp/webgoat/DeserializationTest.java index e1373cf0b..b133d05a2 100644 --- a/webgoat-integration-tests/src/test/java/org/owasp/webgoat/DeserializationTest.java +++ b/webgoat-integration-tests/src/test/java/org/owasp/webgoat/DeserializationTest.java @@ -10,25 +10,25 @@ import org.owasp.webgoat.deserialization.SerializationHelper; public class DeserializationTest extends IntegrationTest { - private static String OS = System.getProperty("os.name").toLowerCase(); - + private static String OS = System.getProperty("os.name").toLowerCase(); + @Test public void runTests() throws IOException { startLesson("InsecureDeserialization"); - + Map params = new HashMap<>(); params.clear(); - - if (OS.indexOf("win")>-1) { - params.put("token", SerializationHelper.toString(new VulnerableTaskHolder("wait", "ping localhost -n 5"))); + + if (OS.indexOf("win") > -1) { + params.put("token", SerializationHelper.toString(new VulnerableTaskHolder("wait", "ping localhost -n 5"))); } else { params.put("token", SerializationHelper.toString(new VulnerableTaskHolder("wait", "sleep 5"))); } - checkAssignment(url("/WebGoat/InsecureDeserialization/task"),params,true); - + checkAssignment(url("/WebGoat/InsecureDeserialization/task"), params, true); + checkResults("/InsecureDeserialization/"); - + } - - + + } 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 fa924e43b..d32ce336e 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 @@ -1,14 +1,7 @@ package org.owasp.webgoat; -import static org.junit.jupiter.api.DynamicTest.dynamicTest; - -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Arrays; -import java.util.Map; - +import io.restassured.RestAssured; +import lombok.SneakyThrows; import org.hamcrest.CoreMatchers; import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.AfterEach; @@ -18,38 +11,49 @@ import org.junit.jupiter.api.TestFactory; import org.junit.jupiter.api.io.TempDir; import org.springframework.security.core.token.Sha512DigestUtils; -import io.restassured.RestAssured; -import lombok.SneakyThrows; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.DynamicTest.dynamicTest; public class PathTraversalTest extends IntegrationTest { - - //the JUnit5 way + + //the JUnit5 way @TempDir Path tempDir; - + private File fileToUpload = null; - + @BeforeEach @SneakyThrows public void init() { - fileToUpload = Files.createFile( + fileToUpload = Files.createFile( tempDir.resolve("test.jpg")).toFile(); - Files.write(fileToUpload.toPath(), "This is a test" .getBytes()); - startLesson("PathTraversal"); + Files.write(fileToUpload.toPath(), "This is a test".getBytes()); + startLesson("PathTraversal"); } @TestFactory Iterable testPathTraversal() { - return Arrays.asList( - dynamicTest("assignment 1 - profile upload",()-> assignment1()), - dynamicTest("assignment 2 - profile upload fix",()-> assignment2()), - dynamicTest("assignment 3 - profile upload remove user input",()-> assignment3()), - dynamicTest("assignment 4 - profile upload random pic",()-> assignment4()) - ); + return Arrays.asList( + dynamicTest("assignment 1 - profile upload", () -> assignment1()), + dynamicTest("assignment 2 - profile upload fix", () -> assignment2()), + dynamicTest("assignment 3 - profile upload remove user input", () -> assignment3()), + dynamicTest("assignment 4 - profile upload random pic", () -> assignment4()), + dynamicTest("assignment 5 - zip slip", () -> assignment5()) + ); } - + public void assignment1() throws IOException { - MatcherAssert.assertThat( + MatcherAssert.assertThat( RestAssured.given() .when() .relaxedHTTPSValidation() @@ -63,7 +67,7 @@ public class PathTraversalTest extends IntegrationTest { } public void assignment2() throws IOException { - MatcherAssert.assertThat( + MatcherAssert.assertThat( RestAssured.given() .when() .relaxedHTTPSValidation() @@ -77,7 +81,7 @@ public class PathTraversalTest extends IntegrationTest { } public void assignment3() throws IOException { - MatcherAssert.assertThat( + MatcherAssert.assertThat( RestAssured.given() .when() .relaxedHTTPSValidation() @@ -88,6 +92,7 @@ public class PathTraversalTest extends IntegrationTest { .statusCode(200) .extract().path("lessonCompleted"), CoreMatchers.is(true)); } + public void assignment4() throws IOException { var uri = "/WebGoat/PathTraversal/random-picture?id=%2E%2E%2F%2E%2E%2Fpath-traversal-secret"; RestAssured.given().urlEncodingEnabled(false) @@ -101,10 +106,34 @@ public class PathTraversalTest extends IntegrationTest { checkAssignment("/WebGoat/PathTraversal/random", Map.of("secret", Sha512DigestUtils.shaHex(getWebgoatUser())), true); } - + + public void assignment5() throws IOException { + var webGoatHome = System.getProperty("user.dir") + "/target/.webgoat/PathTraversal/" + getWebgoatUser(); + webGoatHome = webGoatHome.replaceAll("^[a-zA-Z]:", ""); //Remove C: from the home directory on Windows + + var webGoatDirectory = new File(webGoatHome); + var zipFile = new File(webGoatDirectory, "upload.zip"); + try (var zos = new ZipOutputStream(new FileOutputStream(zipFile))) { + ZipEntry e = new ZipEntry("../../../../../../../../../../" + webGoatDirectory.toString() + "/image.jpg"); + zos.putNextEntry(e); + zos.write("test".getBytes(StandardCharsets.UTF_8)); + } + MatcherAssert.assertThat( + RestAssured.given() + .when() + .relaxedHTTPSValidation() + .cookie("JSESSIONID", getWebGoatCookie()) + .multiPart("uploadedFileZipSlip", "upload.zip", Files.readAllBytes(zipFile.toPath())) + .post("/WebGoat/PathTraversal/zip-slip") + .then() + .statusCode(200) + .extract().path("lessonCompleted"), CoreMatchers.is(true)); + + } + @AfterEach public void shutdown() { - //this will run only once after the list of dynamic tests has run, this is to test if the lesson is marked complete - checkResults("/PathTraversal"); + //this will run only once after the list of dynamic tests has run, this is to test if the lesson is marked complete + checkResults("/PathTraversal"); } } diff --git a/webgoat-lessons/auth-bypass/pom.xml b/webgoat-lessons/auth-bypass/pom.xml index 3578e8972..60a5889a3 100644 --- a/webgoat-lessons/auth-bypass/pom.xml +++ b/webgoat-lessons/auth-bypass/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/bypass-restrictions/pom.xml b/webgoat-lessons/bypass-restrictions/pom.xml index c940b8c63..389fdad1a 100755 --- a/webgoat-lessons/bypass-restrictions/pom.xml +++ b/webgoat-lessons/bypass-restrictions/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/challenge/pom.xml b/webgoat-lessons/challenge/pom.xml index 874053493..8b6ef97b0 100644 --- a/webgoat-lessons/challenge/pom.xml +++ b/webgoat-lessons/challenge/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/chrome-dev-tools/pom.xml b/webgoat-lessons/chrome-dev-tools/pom.xml index d2091f893..1a18a7f84 100644 --- a/webgoat-lessons/chrome-dev-tools/pom.xml +++ b/webgoat-lessons/chrome-dev-tools/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/cia/pom.xml b/webgoat-lessons/cia/pom.xml index 94fe3f9e4..f369dc021 100644 --- a/webgoat-lessons/cia/pom.xml +++ b/webgoat-lessons/cia/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/client-side-filtering/pom.xml b/webgoat-lessons/client-side-filtering/pom.xml index fe0f35b49..f53e5566c 100644 --- a/webgoat-lessons/client-side-filtering/pom.xml +++ b/webgoat-lessons/client-side-filtering/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/cross-site-scripting/pom.xml b/webgoat-lessons/cross-site-scripting/pom.xml index 7a0171373..d1ddeedf2 100644 --- a/webgoat-lessons/cross-site-scripting/pom.xml +++ b/webgoat-lessons/cross-site-scripting/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/crypto/pom.xml b/webgoat-lessons/crypto/pom.xml index 3a5c50e96..db4d7144b 100644 --- a/webgoat-lessons/crypto/pom.xml +++ b/webgoat-lessons/crypto/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/csrf/pom.xml b/webgoat-lessons/csrf/pom.xml index dd0d0566d..e0e27d6f0 100644 --- a/webgoat-lessons/csrf/pom.xml +++ b/webgoat-lessons/csrf/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/html-tampering/pom.xml b/webgoat-lessons/html-tampering/pom.xml index 5478f4126..4fe3c1aa8 100755 --- a/webgoat-lessons/html-tampering/pom.xml +++ b/webgoat-lessons/html-tampering/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/http-basics/pom.xml b/webgoat-lessons/http-basics/pom.xml index 12548a587..d6d5024fe 100644 --- a/webgoat-lessons/http-basics/pom.xml +++ b/webgoat-lessons/http-basics/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/http-proxies/pom.xml b/webgoat-lessons/http-proxies/pom.xml index e17527af8..de9b0e787 100644 --- a/webgoat-lessons/http-proxies/pom.xml +++ b/webgoat-lessons/http-proxies/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/idor/pom.xml b/webgoat-lessons/idor/pom.xml index 6cf15bad7..cbfbcd8b8 100644 --- a/webgoat-lessons/idor/pom.xml +++ b/webgoat-lessons/idor/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/insecure-deserialization/pom.xml b/webgoat-lessons/insecure-deserialization/pom.xml index 7db02049a..5485db034 100755 --- a/webgoat-lessons/insecure-deserialization/pom.xml +++ b/webgoat-lessons/insecure-deserialization/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/insecure-login/pom.xml b/webgoat-lessons/insecure-login/pom.xml index 73f64d793..85654b96c 100755 --- a/webgoat-lessons/insecure-login/pom.xml +++ b/webgoat-lessons/insecure-login/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/jwt/pom.xml b/webgoat-lessons/jwt/pom.xml index 9add3ad70..26f4fca7b 100644 --- a/webgoat-lessons/jwt/pom.xml +++ b/webgoat-lessons/jwt/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/missing-function-ac/pom.xml b/webgoat-lessons/missing-function-ac/pom.xml index c56190289..4fc8c9d81 100644 --- a/webgoat-lessons/missing-function-ac/pom.xml +++ b/webgoat-lessons/missing-function-ac/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/password-reset/pom.xml b/webgoat-lessons/password-reset/pom.xml index f4aa1f5f2..699a1b203 100644 --- a/webgoat-lessons/password-reset/pom.xml +++ b/webgoat-lessons/password-reset/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/path-traversal/pom.xml b/webgoat-lessons/path-traversal/pom.xml index a86048b84..6d065e265 100644 --- a/webgoat-lessons/path-traversal/pom.xml +++ b/webgoat-lessons/path-traversal/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file 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 42a4bdf3f..08923181e 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 @@ -1,6 +1,7 @@ package org.owasp.webgoat.path_traversal; import lombok.AllArgsConstructor; +import lombok.Getter; import lombok.SneakyThrows; import org.owasp.webgoat.assignments.AssignmentEndpoint; import org.owasp.webgoat.assignments.AttackResult; @@ -15,9 +16,12 @@ import org.springframework.web.multipart.MultipartFile; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.util.Arrays; import java.util.Base64; +import java.util.Comparator; @AllArgsConstructor +@Getter public class ProfileUploadBase extends AssignmentEndpoint { private String webGoatHomeDirectory; @@ -64,14 +68,18 @@ public class ProfileUploadBase extends AssignmentEndpoint { } public ResponseEntity getProfilePicture() { + return ResponseEntity.ok() + .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) + .body(getProfilePictureAsBase64()); + } + + protected byte[] getProfilePictureAsBase64() { var profilePictureDirectory = new File(this.webGoatHomeDirectory, "/PathTraversal/" + webSession.getUserName()); var profileDirectoryFiles = profilePictureDirectory.listFiles(); if (profileDirectoryFiles != null && profileDirectoryFiles.length > 0) { try (var inputStream = new FileInputStream(profileDirectoryFiles[0])) { - return ResponseEntity.ok() - .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) - .body(Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream))); + return Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream)); } catch (IOException e) { return defaultImage(); } @@ -81,10 +89,8 @@ public class ProfileUploadBase extends AssignmentEndpoint { } @SneakyThrows - private ResponseEntity defaultImage() { + private byte[] defaultImage() { var inputStream = getClass().getResourceAsStream("/images/account.png"); - return ResponseEntity.ok() - .contentType(MediaType.parseMediaType(MediaType.IMAGE_JPEG_VALUE)) - .body(Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream))); + return Base64.getEncoder().encode(FileCopyUtils.copyToByteArray(inputStream)); } } diff --git a/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileZipSlip.java b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileZipSlip.java new file mode 100644 index 000000000..7bf9239bf --- /dev/null +++ b/webgoat-lessons/path-traversal/src/main/java/org/owasp/webgoat/path_traversal/ProfileZipSlip.java @@ -0,0 +1,93 @@ +package org.owasp.webgoat.path_traversal; + +import lombok.SneakyThrows; +import org.owasp.webgoat.assignments.AssignmentHints; +import org.owasp.webgoat.assignments.AttackResult; +import org.owasp.webgoat.session.WebSession; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.ResponseEntity; +import org.springframework.util.FileCopyUtils; +import org.springframework.util.FileSystemUtils; +import org.springframework.web.bind.annotation.*; +import org.springframework.web.multipart.MultipartFile; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.CopyOption; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.Arrays; +import java.util.Enumeration; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +import static org.springframework.http.MediaType.ALL_VALUE; +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; + +@RestController +@AssignmentHints({"path-traversal-zip-slip.hint1", "path-traversal-zip-slip.hint2", "path-traversal-zip-slip.hint3", "path-traversal-zip-slip.hint4"}) +public class ProfileZipSlip extends ProfileUploadBase { + + public ProfileZipSlip(@Value("${webgoat.server.directory}") String webGoatHomeDirectory, WebSession webSession) { + super(webGoatHomeDirectory, webSession); + } + + @PostMapping(value = "/PathTraversal/zip-slip", consumes = ALL_VALUE, produces = APPLICATION_JSON_VALUE) + @ResponseBody + public AttackResult uploadFileHandler(@RequestParam("uploadedFileZipSlip") MultipartFile file) { + if (!file.getOriginalFilename().toLowerCase().endsWith(".zip")) { + return failed(this).feedback("path-traversal-zip-slip.no-zip").build(); + } else { + return processZipUpload(file); + } + } + + @SneakyThrows + private AttackResult processZipUpload(MultipartFile file) { + var tmpZipDirectory = new File(getWebGoatHomeDirectory(), "/PathTraversal/zip-slip/" + getWebSession().getUserName()); + var uploadDirectory = new File(getWebGoatHomeDirectory(), "/PathTraversal/" + getWebSession().getUserName()); + FileSystemUtils.deleteRecursively(uploadDirectory); + Files.createDirectories(tmpZipDirectory.toPath()); + Files.createDirectories(uploadDirectory.toPath()); + byte[] currentImage = getProfilePictureAsBase64(); + + try { + var uploadedZipFile = new File(tmpZipDirectory, file.getOriginalFilename()); + FileCopyUtils.copy(file.getBytes(), uploadedZipFile); + + ZipFile zip = new ZipFile(uploadedZipFile); + Enumeration entries = zip.entries(); + while (entries.hasMoreElements()) { + ZipEntry e = entries.nextElement(); + File f = new File(uploadDirectory, e.getName()); + InputStream is = zip.getInputStream(e); + Files.copy(is, f.toPath(), StandardCopyOption.REPLACE_EXISTING); + } + + return isSolved(currentImage, getProfilePictureAsBase64()); + } catch (IOException e) { + return failed(this).output(e.getMessage()).build(); + } + } + + 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(); + } + + @GetMapping("/PathTraversal/zip-slip/") + @ResponseBody + public ResponseEntity getProfilePicture() { + return super.getProfilePicture(); + } + + @GetMapping("/PathTraversal/zip-slip/profile-image/{username}") + @ResponseBody + public ResponseEntity getProfilePicture(@PathVariable("username") String username) { + return ResponseEntity.notFound().build(); + } + +} 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 dc40205f7..135be2be6 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/html/PathTraversal.html +++ b/webgoat-lessons/path-traversal/src/main/resources/html/PathTraversal.html @@ -78,7 +78,7 @@ enctype="multipart/form-data" action="/WebGoat/PathTraversal/profile-upload-fix">
- Preview Image
@@ -133,7 +133,7 @@ enctype="multipart/form-data" action="/WebGoat/PathTraversal/profile-upload-remove-user-input">
- Preview Image
@@ -211,4 +211,70 @@
+
+
+
+ +
+
+
+
+
+
+
+ + Preview Image +
+ + +
+ +
+
+ + + +
+
+ + + +
+
+ + + +
+
+ +
+
+
+ +
+
+
+
+
+ +
+
+
+
+
+ \ No newline at end of file diff --git a/webgoat-lessons/path-traversal/src/main/resources/i18n/WebGoatLabels.properties b/webgoat-lessons/path-traversal/src/main/resources/i18n/WebGoatLabels.properties index d960c3c30..cc5dd57ca 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/i18n/WebGoatLabels.properties +++ b/webgoat-lessons/path-traversal/src/main/resources/i18n/WebGoatLabels.properties @@ -45,4 +45,14 @@ path-traversal-profile-retrieve.hint2=Look at the location header... path-traversal-profile-retrieve.hint3=Use /random?id=1 for example to fetch a specific image path-traversal-profile-retrieve.hint4=Use /random/?id=../../1.jpg to navigate to a different directory path-traversal-profile-retrieve.hint5='..' and '/' are no longer allowed, can you bypass this restriction -path-traversal-profile-retrieve.hint6=Use url encoding for ../ to bypass the restriction \ No newline at end of file +path-traversal-profile-retrieve.hint6=Use url encoding for ../ to bypass the restriction + +path-traversal-zip-slip.hint1=Try uploading a picture in a zip file +path-traversal-zip-slip.hint2=Upload a zip file which traverses to the right directory +path-traversal-zip-slip.hint3=Did you create a zip file with the right image name? +path-traversal-zip-slip.hint4=Check the http request to find out which image name should be used + + +path-traversal-zip-slip.no-zip=Please upload a zip file +path-traversal-zip-slip.extracted=Zip file extracted successfully, failed to copy image. Please contact our helpdesk. + diff --git a/webgoat-lessons/path-traversal/src/main/resources/js/path_traversal.js b/webgoat-lessons/path-traversal/src/main/resources/js/path_traversal.js index 740e03012..0c2b341b8 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/js/path_traversal.js +++ b/webgoat-lessons/path-traversal/src/main/resources/js/path_traversal.js @@ -60,3 +60,19 @@ function newRandomPicture() { document.getElementById("randomCatPicture").src = "data:image/png;base64," + result; }); } + +webgoat.customjs.profileZipSlip = function () { + var picture = document.getElementById("uploadedFileZipSlip").files[0]; + var formData = new FormData(); + formData.append("uploadedFileZipSlip", picture); + formData.append("fullName", $("#fullNameZipSlip").val()); + formData.append("email", $("#emailZipSlip").val()); + formData.append("password", $("#passwordZipSlip").val()); + return formData; +} + +webgoat.customjs.profileZipSlipRetrieval = function () { + $.get("PathTraversal/zip-slip", function (result, status) { + document.getElementById("previewZipSlip").src = "data:image/png;base64," + result; + }); +} diff --git a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload.adoc b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload.adoc index 7747b1903..071f44a86 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload.adoc +++ b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload.adoc @@ -7,6 +7,6 @@ so you need to upload your file to the following location which is outside the n |OS |Location |`operatingSystem:os[]` -|`webGoatTempDir:temppath[]PathTraversal/` +|`webGoatTempDir:temppath[]PathTraversal/username:user[]` |=== diff --git a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_fix.adoc b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_fix.adoc index 7bef84ee9..14146ba1e 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_fix.adoc +++ b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_fix.adoc @@ -7,5 +7,5 @@ Again the same assignment but can you bypass the implemented fix? |OS |Location |`operatingSystem:os[]` -|`webGoatTempDir:temppath[]PathTraversal/` +|`webGoatTempDir:temppath[]PathTraversal/username:user[]` |=== diff --git a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_remove_user_input.adoc b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_remove_user_input.adoc index 3a6e2c270..aff3f2cbf 100644 --- a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_remove_user_input.adoc +++ b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_upload_remove_user_input.adoc @@ -9,6 +9,6 @@ Again the same assignment but can you bypass the implemented fix? |OS |Location |`operatingSystem:os[]` -|`webGoatTempDir:temppath[]PathTraversal/` +|`webGoatTempDir:temppath[]PathTraversal/username:user[]` |=== diff --git a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip.adoc b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip.adoc new file mode 100644 index 000000000..aca05e895 --- /dev/null +++ b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip.adoc @@ -0,0 +1,31 @@ +=== Zip Slip vulnerability + +As a developer, you have many occasions where you have to deal with zip files, for example, think about the upload facility or processing a bunch of CSV files that are uploaded as a zip file. A neat vulnerability was discovered and responsibly disclosed by the Snyk Security team. It uses path traversal which can be used while extracting files. With the path traversal, you try to overwrite files outside the intended target folder. For example, you might be able to overwrite the `ls` command while extracting a zip file. Once this command has been replaced with some extra malicious actions each time the user types in `ls` you can for example send the outcome of the listing towards your server before showing the real command to the user. So you end up with remote command execution. + +==== Problem + +The problem occurs with how we extract zip files in Java a common way to do this is: + +[source] +---- +File destinationDir = new File("/tmp/zip"); +Enumeration entries = zip.entries(); +while (entries.hasMoreElements()) { + ZipEntry e = entries.nextElement(); + File f = new File(destinationDir, e.getName()); + InputStream is = zip.getInputStream(e); + IOUtils.copy(is, write(f)); +} +---- + +At first glance, this looks ok and you wrote something along the same lines. The problem is, as we have seen in the previous assignments, that you can use a path traversal to break out of the `destinationDir` and start walking towards different locations. + +But what if we receive a zip file with the following contents: + +[source] +---- +orders.csv +../../../../../../../tmp/evil.sh +---- + +if you extract the zip file with the code above the file will be saved in `/tmp/evil.sh`. \ No newline at end of file diff --git a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip_assignment.adoc b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip_assignment.adoc new file mode 100644 index 000000000..b51b91964 --- /dev/null +++ b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip_assignment.adoc @@ -0,0 +1,13 @@ +=== Zip Slip assignment + +This time the developers only allow you to upload zip files, however, they made a programming mistake in that uploading the zip file will extract it but it will not replace your image. Can you find a way to overwrite your current image bypassing the programming mistake? + +|=== +|OS |Location + +|`operatingSystem:os[]` +|`webGoatTempDir:temppath[]PathTraversal/username:user[]` + +|=== + + diff --git a/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip_solution.adoc b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip_solution.adoc new file mode 100644 index 000000000..dc55affce --- /dev/null +++ b/webgoat-lessons/path-traversal/src/main/resources/lessonPlans/en/PathTraversal_zip_slip_solution.adoc @@ -0,0 +1,56 @@ +=== Solution + +First let's create a zip file with an image inside: + +[source] +---- +curl -o cat.jpg http://localhost:8080/WebGoat/images/cats/1.jpg +zip profile.zip cat.jpg +---- + +Now let's upload this as our profile image, we can see nothing happens as mentioned in the assignment there is a bug in the software and the result we see on the screen is: + +[source] +---- +Zip file extracted successfully, failed to copy image. Please contact our helpdesk. +---- + +Let's create a zip file which traverses all the way to the top and then back into the given directory in the assignment. + +First create the directory structure: + +[source, subs="macros"] +---- +mkdir -p webGoatTempDir:temppath[]PathTraversal/username:user[] +cd webGoatTempDir:temppath[]PathTraversal/username:user[] +curl -o username:user[] http://localhost:8080/WebGoat/images/cats/1.jpg +zip profile.zip ../../../../../../../..webGoatTempDir:temppath[]PathTraversal/username:user[]/username:user[].jpg +---- + +Now if we upload this zip file, the assignment will be solved. + +=== Why did this work? + +In the code the developers used the following fragment: + +[source%linenums] +---- +ZipFile zip = new ZipFile(uploadedZipFile); +Enumeration entries = zip.entries(); +while (entries.hasMoreElements()) { + ZipEntry e = entries.nextElement(); + File profilePicture = new File(uploadDirectory, e.getName()); + InputStream is = zip.getInputStream(e); + Files.copy(is, f.toPath(), StandardCopyOption.REPLACE_EXISTING); +} +---- + +The fix is to make sure the resulting file in line 5 resides in the directory you expect. You can use the following method in Java: + +[source] +---- +File profilePicture = new File(uploadDirectory, e.getName()); +if (profilePicture. + +---- + diff --git a/webgoat-lessons/pom.xml b/webgoat-lessons/pom.xml index fad8120f3..3776e415c 100644 --- a/webgoat-lessons/pom.xml +++ b/webgoat-lessons/pom.xml @@ -5,12 +5,12 @@ org.owasp.webgoat.lesson webgoat-lessons-parent pom - 8.2.0-SNAPSHOT + 8.2.0 org.owasp.webgoat webgoat-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/secure-passwords/pom.xml b/webgoat-lessons/secure-passwords/pom.xml index 8d1e26787..be35d7553 100644 --- a/webgoat-lessons/secure-passwords/pom.xml +++ b/webgoat-lessons/secure-passwords/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/sql-injection/pom.xml b/webgoat-lessons/sql-injection/pom.xml index b1e63bdb4..68dd53690 100644 --- a/webgoat-lessons/sql-injection/pom.xml +++ b/webgoat-lessons/sql-injection/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/ssrf/pom.xml b/webgoat-lessons/ssrf/pom.xml index 15242416c..297931018 100755 --- a/webgoat-lessons/ssrf/pom.xml +++ b/webgoat-lessons/ssrf/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/vulnerable-components/pom.xml b/webgoat-lessons/vulnerable-components/pom.xml index 827c349c1..a2a5365df 100644 --- a/webgoat-lessons/vulnerable-components/pom.xml +++ b/webgoat-lessons/vulnerable-components/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/webgoat-introduction/pom.xml b/webgoat-lessons/webgoat-introduction/pom.xml index 5372e2798..53fd4edf6 100644 --- a/webgoat-lessons/webgoat-introduction/pom.xml +++ b/webgoat-lessons/webgoat-introduction/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/webgoat-lesson-template/pom.xml b/webgoat-lessons/webgoat-lesson-template/pom.xml index cc275b76f..a6940ce55 100644 --- a/webgoat-lessons/webgoat-lesson-template/pom.xml +++ b/webgoat-lessons/webgoat-lesson-template/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-lessons/webwolf-introduction/pom.xml b/webgoat-lessons/webwolf-introduction/pom.xml index ab185a506..953672f51 100644 --- a/webgoat-lessons/webwolf-introduction/pom.xml +++ b/webgoat-lessons/webwolf-introduction/pom.xml @@ -6,6 +6,6 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 \ No newline at end of file diff --git a/webgoat-lessons/xxe/pom.xml b/webgoat-lessons/xxe/pom.xml index 3e2e87a39..8f316274d 100644 --- a/webgoat-lessons/xxe/pom.xml +++ b/webgoat-lessons/xxe/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat.lesson webgoat-lessons-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webgoat-server/pom.xml b/webgoat-server/pom.xml index e3bce069f..69aebaa44 100644 --- a/webgoat-server/pom.xml +++ b/webgoat-server/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat webgoat-parent - 8.2.0-SNAPSHOT + 8.2.0 diff --git a/webwolf/pom.xml b/webwolf/pom.xml index 57507810b..b65e87c45 100644 --- a/webwolf/pom.xml +++ b/webwolf/pom.xml @@ -6,7 +6,7 @@ org.owasp.webgoat webgoat-parent - 8.2.0-SNAPSHOT + 8.2.0