From df889a0f6f6ca8b1bf2db6201cca11ef31b42a13 Mon Sep 17 00:00:00 2001 From: Christophe Tafani-Dereeper Date: Wed, 12 Jun 2024 14:22:25 +0200 Subject: [PATCH] Add arbitrary file read vuln --- Dockerfile | 1 + README.md | 7 ++ .../DomainTestRequest.java | 8 --- .../WebsiteTestRequest.java | 10 --- .../{ => controller}/MainController.java | 26 ++++++- .../exception/DomainTestException.java | 2 - .../exception/FileForbiddenFileException.java | 8 +++ .../exception/FileReadException.java | 8 +++ .../exception/InvalidDomainException.java | 2 - .../http/DomainTestRequest.java | 8 +++ .../http/ViewFileRequest.java | 8 +++ .../http/WebsiteTestRequest.java | 10 +++ .../{ => service}/DomainTestService.java | 3 +- .../service/FileService.java | 31 +++++++++ .../{ => service}/WebsiteTestService.java | 3 +- src/main/resources/static/file.html | 69 +++++++++++++++++++ src/main/resources/static/js/file.js | 37 ++++++++++ 17 files changed, 215 insertions(+), 26 deletions(-) delete mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestRequest.java delete mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestRequest.java rename src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/{ => controller}/MainController.java (56%) create mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileForbiddenFileException.java create mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileReadException.java create mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/DomainTestRequest.java create mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/ViewFileRequest.java create mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/WebsiteTestRequest.java rename src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/{ => service}/DomainTestService.java (95%) create mode 100644 src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/FileService.java rename src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/{ => service}/WebsiteTestService.java (88%) create mode 100644 src/main/resources/static/file.html create mode 100644 src/main/resources/static/js/file.js diff --git a/Dockerfile b/Dockerfile index d0843e4..7d4fb53 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,6 +18,7 @@ RUN wget -O dd-java-agent.jar https://github.com/DataDog/dd-trace-java/releases/ # Utility RUN apk add curl wget +RUN mkdir -p /tmp/files && echo "hello" > /tmp/files/hello.txt && echo "world" > /tmp/files/foo.txt #CMD ["sh", "-c", "export INSTANCE_IP=$(curl http://169.254.169.254/latest/meta-data/local-ipv4); export DD_TRACE_AGENT_URL=http://$INSTANCE_IP:8126; java -javaagent:/app/dd-java-agent.jar -jar /app/spring-boot-application.jar"] CMD ["java", "-javaagent:/app/dd-java-agent.jar", "-jar", "/app/spring-boot-application.jar"] diff --git a/README.md b/README.md index ff1657a..6f8fafa 100644 --- a/README.md +++ b/README.md @@ -41,4 +41,11 @@ You can then access the web application at http://127.0.0.1:8000 3. Note that there is some level of input validation - entering `$(whoami)` returns `Invalid domain name: $(whoami) - don't try to hack us!` 4. However, the validation is buggy - notice how you can start the input with a domain name, and execute and command in the container! +### Local file read vulnerability + +1. Browse to http://127.0.0.1:8000/file.html +2. Note how the input allows you to specify file names such as `/tmp/files/hello.txt` and read them +3. Note that there is some level of input validation - entering `/etc/passwd` returns `You are not allowed to read /etc/passwd` +4. However, the validation is buggy and vulnerable to path traversal. For instance, you can enter `/tmp/files/../../etc/passwd` to bypass the validation and read any file on the local filesystem. + ![image](https://user-images.githubusercontent.com/136675/186954376-e3d82d03-7d9e-49b3-a106-6da080980dae.png) diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestRequest.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestRequest.java deleted file mode 100644 index 4969765..0000000 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestRequest.java +++ /dev/null @@ -1,8 +0,0 @@ -package com.datadoghq.workshops.samplevulnerablejavaapp; - -import lombok.Data; - -@Data -public class DomainTestRequest { - String domainName; -} diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestRequest.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestRequest.java deleted file mode 100644 index e57c7b1..0000000 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestRequest.java +++ /dev/null @@ -1,10 +0,0 @@ -package com.datadoghq.workshops.samplevulnerablejavaapp; - -import lombok.Data; - -@Data -public class WebsiteTestRequest { - String url; - String customHeaderKey; - String customHeaderValue; -} diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/MainController.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/controller/MainController.java similarity index 56% rename from src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/MainController.java rename to src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/controller/MainController.java index d8bb246..bb3721a 100644 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/MainController.java +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/controller/MainController.java @@ -1,7 +1,15 @@ -package com.datadoghq.workshops.samplevulnerablejavaapp; +package com.datadoghq.workshops.samplevulnerablejavaapp.controller; +import com.datadoghq.workshops.samplevulnerablejavaapp.exception.FileForbiddenFileException; +import com.datadoghq.workshops.samplevulnerablejavaapp.exception.FileReadException; import com.datadoghq.workshops.samplevulnerablejavaapp.exception.InvalidDomainException; import com.datadoghq.workshops.samplevulnerablejavaapp.exception.UnableToTestDomainException; +import com.datadoghq.workshops.samplevulnerablejavaapp.http.DomainTestRequest; +import com.datadoghq.workshops.samplevulnerablejavaapp.http.ViewFileRequest; +import com.datadoghq.workshops.samplevulnerablejavaapp.http.WebsiteTestRequest; +import com.datadoghq.workshops.samplevulnerablejavaapp.service.DomainTestService; +import com.datadoghq.workshops.samplevulnerablejavaapp.service.FileService; +import com.datadoghq.workshops.samplevulnerablejavaapp.service.WebsiteTestService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -20,6 +28,9 @@ public class MainController { @Autowired private WebsiteTestService websiteTestService; + @Autowired + private FileService fileService; + @RequestMapping(method=RequestMethod.POST, value="/test-domain", consumes="application/json") public ResponseEntity testDomain(@RequestBody DomainTestRequest request) { log.info("Testing domain " + request.domainName); @@ -42,4 +53,17 @@ public ResponseEntity testWebsite(@RequestBody WebsiteTestRequest reques return new ResponseEntity<>(result, HttpStatus.OK); } + @RequestMapping(method=RequestMethod.POST, value="/view-file", consumes="application/json") + public ResponseEntity viewFile(@RequestBody ViewFileRequest request) { + log.info("Reading file " + request.path); + try { + String result = fileService.readFile(request.path); + return new ResponseEntity<>(result, HttpStatus.OK); + } catch (FileForbiddenFileException e) { + return new ResponseEntity<>(e.getMessage(), HttpStatus.FORBIDDEN); + } catch (FileReadException e) { + return new ResponseEntity<>(e.getMessage(), HttpStatus.BAD_REQUEST); + } + } + } diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/DomainTestException.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/DomainTestException.java index ff90b91..ef6e18f 100644 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/DomainTestException.java +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/DomainTestException.java @@ -1,7 +1,5 @@ package com.datadoghq.workshops.samplevulnerablejavaapp.exception; -import lombok.experimental.StandardException; - public class DomainTestException extends Exception { public DomainTestException(String message) { super(message); diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileForbiddenFileException.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileForbiddenFileException.java new file mode 100644 index 0000000..bda2fe2 --- /dev/null +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileForbiddenFileException.java @@ -0,0 +1,8 @@ +package com.datadoghq.workshops.samplevulnerablejavaapp.exception; + +public class FileForbiddenFileException extends Exception { + public FileForbiddenFileException(String message) { + super(message); + } +} + diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileReadException.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileReadException.java new file mode 100644 index 0000000..9e48bba --- /dev/null +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/FileReadException.java @@ -0,0 +1,8 @@ +package com.datadoghq.workshops.samplevulnerablejavaapp.exception; + +public class FileReadException extends Exception { + public FileReadException(String message) { + super(message); + } +} + diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/InvalidDomainException.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/InvalidDomainException.java index 9fe1d03..7a96205 100644 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/InvalidDomainException.java +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/exception/InvalidDomainException.java @@ -1,7 +1,5 @@ package com.datadoghq.workshops.samplevulnerablejavaapp.exception; -import com.datadoghq.workshops.samplevulnerablejavaapp.exception.DomainTestException; - public class InvalidDomainException extends DomainTestException { public InvalidDomainException(String message) { super(message); diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/DomainTestRequest.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/DomainTestRequest.java new file mode 100644 index 0000000..a5f2de4 --- /dev/null +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/DomainTestRequest.java @@ -0,0 +1,8 @@ +package com.datadoghq.workshops.samplevulnerablejavaapp.http; + +import lombok.Data; + +@Data +public class DomainTestRequest { + public String domainName; +} diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/ViewFileRequest.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/ViewFileRequest.java new file mode 100644 index 0000000..2039c8d --- /dev/null +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/ViewFileRequest.java @@ -0,0 +1,8 @@ +package com.datadoghq.workshops.samplevulnerablejavaapp.http; + +import lombok.Data; + +@Data +public class ViewFileRequest { + public String path; +} diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/WebsiteTestRequest.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/WebsiteTestRequest.java new file mode 100644 index 0000000..ca57f26 --- /dev/null +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/http/WebsiteTestRequest.java @@ -0,0 +1,10 @@ +package com.datadoghq.workshops.samplevulnerablejavaapp.http; + +import lombok.Data; + +@Data +public class WebsiteTestRequest { + public String url; + public String customHeaderKey; + public String customHeaderValue; +} diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestService.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/DomainTestService.java similarity index 95% rename from src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestService.java rename to src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/DomainTestService.java index f3f1e10..eb40128 100644 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestService.java +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/DomainTestService.java @@ -1,4 +1,4 @@ -package com.datadoghq.workshops.samplevulnerablejavaapp; +package com.datadoghq.workshops.samplevulnerablejavaapp.service; import com.datadoghq.workshops.samplevulnerablejavaapp.exception.DomainTestException; import com.datadoghq.workshops.samplevulnerablejavaapp.exception.InvalidDomainException; @@ -6,7 +6,6 @@ import org.springframework.stereotype.Service; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/FileService.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/FileService.java new file mode 100644 index 0000000..083ba75 --- /dev/null +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/FileService.java @@ -0,0 +1,31 @@ +package com.datadoghq.workshops.samplevulnerablejavaapp.service; + +import com.datadoghq.workshops.samplevulnerablejavaapp.exception.FileForbiddenFileException; +import com.datadoghq.workshops.samplevulnerablejavaapp.exception.FileReadException; +import org.springframework.stereotype.Service; + +import java.io.*; + +@Service +public class FileService { + final static String ALLOWED_PREFIX = "/tmp/files/"; + + public String readFile(String path) throws FileForbiddenFileException, FileReadException { + if(!path.startsWith(ALLOWED_PREFIX)) { + throw new FileForbiddenFileException("You are not allowed to read " + path); + } + try (BufferedReader br = new BufferedReader(new FileReader(path))) { + StringBuilder sb = new StringBuilder(); + String line = br.readLine(); + + while (line != null) { + sb.append(line); + sb.append(System.lineSeparator()); + line = br.readLine(); + } + return sb.toString(); + } catch (IOException e) { + throw new FileReadException(e.getMessage()); + } + } +} diff --git a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestService.java b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/WebsiteTestService.java similarity index 88% rename from src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestService.java rename to src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/WebsiteTestService.java index 359364f..3901dc6 100644 --- a/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/WebsiteTestService.java +++ b/src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/service/WebsiteTestService.java @@ -1,5 +1,6 @@ -package com.datadoghq.workshops.samplevulnerablejavaapp; +package com.datadoghq.workshops.samplevulnerablejavaapp.service; +import com.datadoghq.workshops.samplevulnerablejavaapp.http.WebsiteTestRequest; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; diff --git a/src/main/resources/static/file.html b/src/main/resources/static/file.html new file mode 100644 index 0000000..e353c32 --- /dev/null +++ b/src/main/resources/static/file.html @@ -0,0 +1,69 @@ + + + + + + + File viewer + + +
+ + + +
+
+

View files on this server. For security reasons, you can only access samples files under /tmp/files.

+ +

Available files: +

    +
  • /tmp/files/hello.txt
  • +
  • /tmp/files/foo.txt
  • +
+

+
+
+ + +
+
+
+ +
+ +
+
+ + +
+
+ +
+ +
+ +
+ +
+
+ + + + + + + + + diff --git a/src/main/resources/static/js/file.js b/src/main/resources/static/js/file.js new file mode 100644 index 0000000..95d840f --- /dev/null +++ b/src/main/resources/static/js/file.js @@ -0,0 +1,37 @@ +var outputContainer = document.getElementById('output-container') +var outputElement = document.getElementById('output') +var errorContainer = document.getElementById('error-container') +var errorElement = document.getElementById('error') + +function updateOutput(result) { + // If there is any error, hide it first + errorContainer.classList.add('hidden'); + outputElement.innerText = result; + outputContainer.classList.remove('hidden'); +} + +function handleError(error) { + // If there is any successful output, hide it first + outputContainer.classList.add('hidden'); + errorElement.innerText = error.responseText; + errorContainer.classList.remove('hidden'); +} + + +function submitRequest() { + $.ajax({ + url: '/view-file', + method: 'POST', + contentType: 'application/json', + accept: 'application/json', + data: JSON.stringify({ + 'path': document.getElementById('path').value || '' + }), + success: updateOutput, + error: handleError + }) +} + + +var form = document.querySelectorAll('form')[0] +form.addEventListener('submit', function(evt) { evt.preventDefault(); submitRequest(); })