Expand SSRF support in IAST to apache-httpclient-5 and apache-httpasy…

…ncclient-4 (#7920)
DataDog · Dec 9, 2024 · 9ac8ab1 · 9ac8ab1
1 parent 4df0a01
commit 9ac8ab1
Show file tree

Hide file tree

Showing 17 changed files with 330 additions and 11 deletions.
diff --git a/...a/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientDecorator.java b/...a/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientDecorator.java
@@ -41,6 +41,11 @@ protected URI url(final HttpUriRequest request) throws URISyntaxException {
     return request.getURI();
   }
 
+  @Override
+  protected URI sourceUrl(final HttpUriRequest request) {
+    return request.getURI();
+  }
+
   @Override
   protected int status(final HttpContext context) {
     final Object responseObject = context.getAttribute(HttpCoreContext.HTTP_RESPONSE);

diff --git a/.../main/java/datadog/trace/instrumentation/apachehttpclient5/ApacheHttpClientDecorator.java b/.../main/java/datadog/trace/instrumentation/apachehttpclient5/ApacheHttpClientDecorator.java
@@ -42,6 +42,11 @@ protected URI url(final HttpRequest request) throws URISyntaxException {
     return request.getUri();
   }
 
+  @Override
+  protected HttpRequest sourceUrl(final HttpRequest request) {
+    return request;
+  }
+
   @Override
   protected int status(final HttpResponse httpResponse) {
     return httpResponse.getCode();

diff --git a/.../java/datadog/trace/instrumentation/apachehttpclient5/HostAndRequestAsHttpUriRequest.java b/.../java/datadog/trace/instrumentation/apachehttpclient5/HostAndRequestAsHttpUriRequest.java
@@ -1,5 +1,6 @@
 package datadog.trace.instrumentation.apachehttpclient5;
 
+import datadog.trace.api.iast.util.PropagationUtils;
 import java.net.URI;
 import java.net.URISyntaxException;
 import org.apache.hc.core5.http.Header;
@@ -15,6 +16,9 @@ public class HostAndRequestAsHttpUriRequest extends BasicClassicHttpRequest {
   public HostAndRequestAsHttpUriRequest(final HttpHost httpHost, final HttpRequest httpRequest) {
     super(httpRequest.getMethod(), httpHost, httpRequest.getPath());
     actualRequest = httpRequest;
+    // Propagate in case the host or request is tainted
+    PropagationUtils.taintObjectIfTainted(this, httpHost);
+    PropagationUtils.taintObjectIfTainted(this, httpRequest);
   }
 
   @Override

diff --git a/...atadog/trace/instrumentation/apachehttpclient5/IastHttpUriRequestBaseInstrumentation.java b/...atadog/trace/instrumentation/apachehttpclient5/IastHttpUriRequestBaseInstrumentation.java
@@ -0,0 +1,53 @@
+package datadog.trace.instrumentation.apachehttpclient5;
+
+import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.agent.tooling.bytebuddy.iast.TaintableVisitor;
+import datadog.trace.api.iast.InstrumentationBridge;
+import datadog.trace.api.iast.Propagation;
+import datadog.trace.api.iast.propagation.PropagationModule;
+import java.net.URI;
+import net.bytebuddy.asm.Advice;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
+
+@AutoService(InstrumenterModule.class)
+public class IastHttpUriRequestBaseInstrumentation extends InstrumenterModule.Iast
+    implements Instrumenter.ForSingleType, Instrumenter.HasTypeAdvice {
+
+  public IastHttpUriRequestBaseInstrumentation() {
+    super("apache-httpclient", "httpclient5");
+  }
+
+  @Override
+  public String instrumentedType() {
+    return "org.apache.hc.client5.http.classic.methods.HttpUriRequestBase";
+  }
+
+  @Override
+  public void typeAdvice(TypeTransformer transformer) {
+    transformer.applyAdvice(new TaintableVisitor(instrumentedType()));
+  }
+
+  @Override
+  public void methodAdvice(MethodTransformer transformer) {
+    transformer.applyAdvice(
+        isConstructor().and(takesArguments(String.class, URI.class)),
+        IastHttpUriRequestBaseInstrumentation.class.getName() + "$CtorAdvice");
+  }
+
+  public static class CtorAdvice {
+    @Advice.OnMethodExit()
+    @Propagation
+    public static void afterCtor(
+        @Advice.This final HttpUriRequestBase self, @Advice.Argument(1) final URI uri) {
+      final PropagationModule module = InstrumentationBridge.PROPAGATION;
+      if (module != null) {
+        module.taintObjectIfTainted(self, uri);
+      }
+    }
+  }
+}
diff --git a/...tion/apache-httpclient-5/src/test/groovy/IastHttpUriRequestBaseInstrumentationTest.groovy b/...tion/apache-httpclient-5/src/test/groovy/IastHttpUriRequestBaseInstrumentationTest.groovy
@@ -0,0 +1,29 @@
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.iast.InstrumentationBridge
+import datadog.trace.api.iast.propagation.PropagationModule
+import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase
+
+class IastHttpUriRequestBaseInstrumentationTest extends AgentTestRunner {
+
+  @Override
+  protected void configurePreAgent() {
+    injectSysConfig('dd.iast.enabled', 'true')
+  }
+
+  void 'test constructor'() {
+    given:
+    final module = Mock(PropagationModule)
+    InstrumentationBridge.registerIastModule(module)
+
+    when:
+    HttpUriRequestBase.newInstance(method, new URI(uri))
+
+    then:
+    1 * module.taintObjectIfTainted(_ as HttpUriRequestBase, _ as URI)
+    0 * _
+
+    where:
+    method | uri
+    "GET"  | 'http://localhost.com'
+  }
+}
diff --git a/dd-java-agent/instrumentation/apache-httpcore-5/build.gradle b/dd-java-agent/instrumentation/apache-httpcore-5/build.gradle
@@ -0,0 +1,20 @@
+muzzle {
+  pass {
+    group = "org.apache.httpcomponents.core5"
+    module = "httpcore5"
+    versions = "[5.0,)"
+    assertInverse = true
+  }
+}
+
+apply from: "$rootDir/gradle/java.gradle"
+
+addTestSuiteForDir('latestDepTest', 'test')
+
+dependencies {
+  compileOnly group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '5.0'
+
+  testImplementation group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '5.0'
+
+  latestDepTestImplementation group: 'org.apache.httpcomponents.core5', name: 'httpcore5', version: '+'
+}
diff --git a/.../main/java/datadog/trace/instrumentation/apachehttpcore5/IastHttpHostInstrumentation.java b/.../main/java/datadog/trace/instrumentation/apachehttpcore5/IastHttpHostInstrumentation.java
@@ -0,0 +1,47 @@
+package datadog.trace.instrumentation.apachehttpcore5;
+
+import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.api.iast.InstrumentationBridge;
+import datadog.trace.api.iast.Propagation;
+import datadog.trace.api.iast.propagation.PropagationModule;
+import java.net.InetAddress;
+import net.bytebuddy.asm.Advice;
+
+@AutoService(InstrumenterModule.class)
+public class IastHttpHostInstrumentation extends InstrumenterModule.Iast
+    implements Instrumenter.ForSingleType {
+
+  public IastHttpHostInstrumentation() {
+    super("httpcore-5", "apache-httpcore-5", "apache-http-core-5");
+  }
+
+  @Override
+  public String instrumentedType() {
+    return "org.apache.hc.core5.http.HttpHost";
+  }
+
+  @Override
+  public void methodAdvice(MethodTransformer transformer) {
+    transformer.applyAdvice(
+        isConstructor()
+            .and(takesArguments(String.class, InetAddress.class, String.class, int.class)),
+        IastHttpHostInstrumentation.class.getName() + "$CtorAdvice");
+  }
+
+  public static class CtorAdvice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    @Propagation
+    public static void afterCtor(
+        @Advice.This final Object self, @Advice.Argument(2) final String host) {
+      final PropagationModule module = InstrumentationBridge.PROPAGATION;
+      if (module != null) {
+        module.taintObjectIfTainted(self, host);
+      }
+    }
+  }
+}
diff --git a/...oovy/datadog/trace/instrumentation/apachehttpcore5/IastHttpHostInstrumentationTest.groovy b/...oovy/datadog/trace/instrumentation/apachehttpcore5/IastHttpHostInstrumentationTest.groovy
@@ -0,0 +1,31 @@
+package datadog.trace.instrumentation.apachehttpcore5
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.iast.InstrumentationBridge
+import datadog.trace.api.iast.propagation.PropagationModule
+import org.apache.hc.core5.http.HttpHost
+
+class IastHttpHostInstrumentationTest extends AgentTestRunner {
+
+  @Override
+  protected void configurePreAgent() {
+    injectSysConfig('dd.iast.enabled', 'true')
+  }
+
+  void 'test constructor'(){
+    given:
+    final module = Mock(PropagationModule)
+    InstrumentationBridge.registerIastModule(module)
+
+    when:
+    HttpHost.newInstance(*args)
+
+    then:
+    1 * module.taintObjectIfTainted( _ as HttpHost, 'localhost')
+
+    where:
+    args | _
+    ['localhost'] | _
+    ['localhost', 8080] | _
+  }
+}
diff --git a/...ter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie b/...ter/src/main/resources/datadog/trace/instrumentation/iastinstrumenter/iast_exclusion.trie
@@ -198,6 +198,7 @@
 1 org.apache.*
 #apache httpClient needs URI propagation
 0 org.apache.http.client.methods.*
+0 org.apache.hc.client5.http.classic.methods.*
 # apache compiled jsps
 0 org.apache.jsp.*
 1 org.apiguardian.*

diff --git a/dd-smoke-tests/iast-util/build.gradle b/dd-smoke-tests/iast-util/build.gradle
@@ -17,4 +17,6 @@ dependencies {
   compileOnly group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
   compileOnly group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0'
   compileOnly group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
+  compileOnly group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.0'
+  compileOnly group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'
 }
diff --git a/...tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/SsrfController.java b/...tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/SsrfController.java
@@ -7,10 +7,16 @@
 import org.apache.commons.httpclient.HttpClient;
 import org.apache.commons.httpclient.HttpMethod;
 import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.http.HttpHost;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
+import org.apache.http.impl.nio.client.HttpAsyncClients;
 import org.apache.http.message.BasicHttpRequest;
+import org.apache.http.nio.client.methods.HttpAsyncMethods;
+import org.apache.http.nio.protocol.HttpAsyncRequestProducer;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -89,4 +95,60 @@ public String okHttp3(@RequestParam(value = "url") final String url) {
     client.connectionPool().evictAll();
     return "ok";
   }
+
+  @PostMapping("/apache-httpclient5")
+  public String apacheHttpClient5(
+      @RequestParam(value = "url", required = false) final String url,
+      @RequestParam(value = "urlHandler", required = false) final String urlHandler,
+      @RequestParam(value = "host", required = false) final String host) {
+    CloseableHttpClient client = HttpClients.createDefault();
+    try {
+      if (host != null) {
+        final org.apache.hc.core5.http.HttpHost httpHost =
+            new org.apache.hc.core5.http.HttpHost(host);
+        final org.apache.hc.client5.http.classic.methods.HttpGet request =
+            new org.apache.hc.client5.http.classic.methods.HttpGet("/");
+        client.execute(httpHost, request);
+      } else if (url != null) {
+        final org.apache.hc.client5.http.classic.methods.HttpGet request =
+            new org.apache.hc.client5.http.classic.methods.HttpGet(url);
+        client.execute(request);
+      } else if (urlHandler != null) {
+        final org.apache.hc.client5.http.classic.methods.HttpGet request =
+            new org.apache.hc.client5.http.classic.methods.HttpGet(urlHandler);
+        client.execute(request, response -> null);
+      }
+      client.close();
+    } catch (Exception e) {
+    }
+    return "ok";
+  }
+
+  @PostMapping("/apache-httpasyncclient")
+  public String apacheHttpAsyncClient(
+      @RequestParam(value = "url", required = false) final String url,
+      @RequestParam(value = "host", required = false) final String host,
+      @RequestParam(value = "urlProducer", required = false) final String urlProducer) {
+    final CloseableHttpAsyncClient client = HttpAsyncClients.createDefault();
+    client.start();
+    try {
+      if (host != null) {
+        final HttpHost httpHost = new HttpHost(host);
+        client.execute(httpHost, new HttpGet("/"), null);
+      } else if (url != null) {
+        final HttpGet request = new HttpGet(url);
+        client.execute(request, null);
+      } else if (urlProducer != null) {
+        final HttpAsyncRequestProducer producer = HttpAsyncMethods.create(new HttpGet(urlProducer));
+        client.execute(producer, null, null);
+      }
+    } catch (Exception e) {
+    } finally {
+      try {
+        client.close();
+      } catch (Exception e) {
+      }
+    }
+    return "ok";
+  }
 }
diff --git a/...sts/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy b/...sts/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy
@@ -724,7 +724,7 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
     'host'    | 'dd.datad0g.com'
   }
 
-  void 'ssrf is present (#path)'() {
+  void 'ssrf is present (#path) (#parameter)'() {
     setup:
     final url = "http://localhost:${httpPort}/ssrf/${path}"
     final body = new FormBody.Builder().add(parameter, value).build()
@@ -744,21 +744,29 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
         && parts[0].value == value && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
       } else if (parameter == 'host') {
         String protocol = protocolSecure ? 'https://' : 'http://'
-        return parts.size() == 2
-        && parts[0].value == protocol + value && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
-        && parts[1].value == '/' && parts[1].source == null
+        String finalValue = protocol + value + (endSlash ? '/' : '')
+        return parts[0].value.endsWith(finalValue) && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
+      } else if (parameter == 'urlProducer' || parameter == 'urlHandler') {
+        return parts.size() == 1
+        && parts[0].value.endsWith(value) && parts[0].source.origin == 'http.request.parameter' && parts[0].source.name == parameter
       } else {
         throw new IllegalArgumentException("Parameter $parameter not supported")
       }
     }
 
     where:
-    path                  | parameter | value                     | protocolSecure
-    "apache-httpclient4"  | "url"     | "https://dd.datad0g.com/" | true
-    "apache-httpclient4"  | "host"    | "dd.datad0g.com"          | false
-    "commons-httpclient2" | "url"     | "https://dd.datad0g.com/" | true
-    "okHttp2"             | "url"     | "https://dd.datad0g.com/" | true
-    "okHttp3"             | "url"     | "https://dd.datad0g.com/" | true
+    path                     | parameter     | value                     | protocolSecure | endSlash
+    "apache-httpclient4"     | "url"         | "https://dd.datad0g.com/" | true           | true
+    "apache-httpclient4"     | "host"        | "dd.datad0g.com"          | false          | false
+    "apache-httpasyncclient" | "url"         | "https://dd.datad0g.com/" | true           | true
+    "apache-httpasyncclient" | "urlProducer" | "https://dd.datad0g.com/" | true           | true
+    "apache-httpasyncclient" | "host"        | "dd.datad0g.com"          | false          | false
+    "apache-httpclient5"     | "url"         | "https://dd.datad0g.com/" | true           | true
+    "apache-httpclient5"     | "urlHandler"  | "https://dd.datad0g.com/" | true           | true
+    "apache-httpclient5"     | "host"        | "dd.datad0g.com"          | false          | true
+    "commons-httpclient2"    | "url"         | "https://dd.datad0g.com/" | true           | true
+    "okHttp2"                | "url"         | "https://dd.datad0g.com/" | true           | true
+    "okHttp3"                | "url"         | "https://dd.datad0g.com/" | true           | true
   }
 
   void 'test iast metrics stored in spans'() {

diff --git a/dd-smoke-tests/spring-boot-2.6-webmvc/build.gradle b/dd-smoke-tests/spring-boot-2.6-webmvc/build.gradle
@@ -42,6 +42,8 @@ dependencies {
   implementation group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
   implementation group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0'
   implementation group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
+  implementation group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.0'
+  implementation group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'
 
   testImplementation project(':dd-smoke-tests')
   implementation project(':dd-smoke-tests:iast-util')

diff --git a/dd-smoke-tests/springboot/build.gradle b/dd-smoke-tests/springboot/build.gradle
@@ -33,7 +33,8 @@ dependencies {
   implementation group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.0'
   implementation group: 'com.squareup.okhttp', name: 'okhttp', version: '2.2.0'
   implementation group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
-
+  implementation group: 'org.apache.httpcomponents.client5', name: 'httpclient5', version: '5.0'
+  implementation group: 'org.apache.httpcomponents', name: 'httpasyncclient', version: '4.0'
 
   testImplementation project(':dd-smoke-tests')
   testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))