Fix Command Injection Vulnerability

src/main/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestService.java

@@ -6,43 +6,30 @@
 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;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 
 @Service
 public class DomainTestService {
 
   final static int timeoutMs = 10_000;
-  final static Pattern domainValidationRegex = Pattern.compile("^((?!-))(xn--)?[a-z0-9][a-z0-9-_]{0,61}[a-z0-9]{0,1}\\.(xn--)?([a-z0-9\\-]{1,61}|[a-z0-9-]{1,30}\\.[a-z]{2,})", Pattern.CASE_INSENSITIVE);
 
   public String testDomain(String domainName) throws DomainTestException {
-    if (!isValidDomainName(domainName)) {
+    InetAddress address;
+    try {
+      address = InetAddress.getByName(domainName);
+    } catch (UnknownHostException e) {
       throw new InvalidDomainException("Invalid domain name: " + domainName + " - don't try to hack us!");
     }
 
     try {
-      //TODO use ProcessBuilder which looks cleaner
-      Process process = Runtime.getRuntime().exec(new String[] {"sh", "-c", "ping -c 1 " + domainName});
-      if (!process.waitFor(timeoutMs, TimeUnit.MILLISECONDS)) {
-        throw new UnableToTestDomainException("Timed out pinging domain");
-      }
-      int exitCode = process.exitValue();
-      if (exitCode != 0) {
-        String stderr = new String(process.getErrorStream().readAllBytes(), StandardCharsets.UTF_8);
-        throw new UnableToTestDomainException("Ping returned exit status " + exitCode + ": " + stderr);
+      boolean reachable = address.isReachable(timeoutMs);
+      if (!reachable) {
+        throw new UnableToTestDomainException("The domain " + domainName + "is not reachable!");
       }
-      return new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
+      return "The domain " + domainName + "is reachable!";
     } catch (IOException e) {
       throw new UnableToTestDomainException("Internal error while testing domain: " + e.getMessage());
-    } catch (InterruptedException e) {
-      throw new UnableToTestDomainException("Timed out pinging domain");
     }
   }
-
-  static boolean isValidDomainName(String domainName) {
-    Matcher matcher = domainValidationRegex.matcher(domainName);
-    return matcher.find();
-  }
 }

src/test/java/com/datadoghq/workshops/samplevulnerablejavaapp/DomainTestServiceTests.java

@@ -1,18 +1,30 @@
 package com.datadoghq.workshops.samplevulnerablejavaapp;
 
+import com.datadoghq.workshops.samplevulnerablejavaapp.exception.DomainTestException;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class DomainTestServiceTests {
     @Test
     void testValidDomain() {
-        String domain = "google.com";
-        Assertions.assertTrue(DomainTestService.isValidDomainName(domain));
+        String domain = "localhost";
+        DomainTestService testService = new DomainTestService();
+
+        try {
+            testService.testDomain(domain);
+        } catch (DomainTestException e) {
+            Assertions.fail();
+        }
     }
 
     @Test
     void testInvalidDomain() {
         String domain = "exec script.sh";
-        Assertions.assertFalse(DomainTestService.isValidDomainName(domain));
+        DomainTestService testService = new DomainTestService();
+
+        try {
+            testService.testDomain(domain);
+            Assertions.fail();
+        } catch (DomainTestException ignored) { }
     }
 }