Added exception catches for the OkHTTPClient header vulnerability

communication/src/main/java/datadog/communication/http/OkHttpUtils.java

@@ -153,9 +153,8 @@ private static OkHttpClient buildHttpClient(
               public Request authenticate(final Route route, final Response response) {
                 final String credential =
                     Credentials.basic(proxyUsername, proxyPassword == null ? "" : proxyPassword);
-                return response
-                    .request()
-                    .newBuilder()
+
+                return new SafeRequestBuilder.Builder(response.request())
                     .header("Proxy-Authorization", credential)
                     .build();
               }
@@ -174,14 +173,21 @@ public Request authenticate(final Route route, final Response response) {
     return client;
   }
 
+  /*
+  SafeRequestBuilder adapter is used here to safely add headers
+  It catches the exception because of a vulnerability in okhttp3 that can cause secrets
+  to be printed/logged when invalid characters are passed into the .header() and .addHeader()
+  methods. This just throws an empty IllegalArgumentException without the message instead.
+  */
   public static Request.Builder prepareRequest(final HttpUrl url, Map<String, String> headers) {
-    final Request.Builder builder =
-        new Request.Builder()
-            .url(url)
-            .addHeader(DATADOG_META_LANG, "java")
-            .addHeader(DATADOG_META_LANG_VERSION, JAVA_VERSION)
-            .addHeader(DATADOG_META_LANG_INTERPRETER, JAVA_VM_NAME)
-            .addHeader(DATADOG_META_LANG_INTERPRETER_VENDOR, JAVA_VM_VENDOR);
+
+    final SafeRequestBuilder.Builder builder = new SafeRequestBuilder.Builder();
+    builder
+        .url(url)
+        .addHeader(DATADOG_META_LANG, "java")
+        .addHeader(DATADOG_META_LANG_VERSION, JAVA_VERSION)
+        .addHeader(DATADOG_META_LANG_INTERPRETER, JAVA_VM_NAME)
+        .addHeader(DATADOG_META_LANG_INTERPRETER_VENDOR, JAVA_VM_VENDOR);
 
     final String containerId = ContainerInfo.get().getContainerId();
     if (containerId != null) {
@@ -192,7 +198,20 @@ public static Request.Builder prepareRequest(final HttpUrl url, Map<String, Stri
       builder.addHeader(e.getKey(), e.getValue());
     }
 
-    return builder;
+    return builder.getBuilder();
+  }
+  // This method is used for the one addHeader() call in prepareRequest below.
+  // This catches the Illegal argument to prevent printing/logging header secrets.
+  private static void safeAddHeader(Request.Builder builder, String key, String value) {
+    try {
+      builder.addHeader(key, value);
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException(
+          new StringBuilder()
+              .append("InvalidArgumentException at header() for header: ")
+              .append(key)
+              .toString());
+    }
   }
 
   public static Request.Builder prepareRequest(
@@ -206,7 +225,7 @@ public static Request.Builder prepareRequest(
     if (agentless && apiKey != null) {
       // we only add the api key header if we know we're doing agentless. No point in adding it to
       // other agent-based requests since we know the datadog-agent isn't going to make use of it.
-      builder.addHeader(DD_API_KEY, apiKey);
+      safeAddHeader(builder, DD_API_KEY, apiKey);
     }
 
     return builder;

communication/src/main/java/datadog/communication/http/SafeRequestBuilder.java

@@ -0,0 +1,130 @@
+package datadog.communication.http;
+
+import java.net.URL;
+import javax.annotation.Nullable;
+import okhttp3.CacheControl;
+import okhttp3.Headers;
+import okhttp3.HttpUrl;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+
+/*
+The purpose of this class is to wrap okhttp3.Request.Builder.
+This adapter is used to resolve a vulnerability in Request.builder where
+the .header() and .addHeader() methods can cause header secrets to be
+printed/logged when invalid characters are parsed.
+ */
+public final class SafeRequestBuilder {
+  public static class Builder {
+    private final Request.Builder requestBuilder;
+
+    public Builder() {
+      this.requestBuilder = new Request.Builder();
+    }
+
+    // Constructs a SafeRequestBuilder from an existing request
+    public Builder(Request request) {
+      this.requestBuilder = request.newBuilder();
+    }
+
+    public Request.Builder url(HttpUrl url) {
+      return this.requestBuilder.url(url);
+    }
+
+    public Request.Builder url(String url) {
+      return this.requestBuilder.url(url);
+    }
+
+    public Request.Builder url(URL url) {
+      return this.requestBuilder.url(url);
+    }
+
+    public Request.Builder header(String name, String value) {
+      // This try/catch block prevents header secrets from being outputted to
+      // console or logs.
+      try {
+        return this.requestBuilder.header(name, value);
+      } catch (IllegalArgumentException e) {
+        throw new IllegalArgumentException(
+            new StringBuilder()
+                .append("InvalidArgumentException at header() for header: ")
+                .append(name)
+                .toString());
+      }
+    }
+
+    public Request.Builder addHeader(String name, String value) {
+      // This try/catch block prevents header secrets from being outputted to
+      // console or logs.
+      try {
+        return this.requestBuilder.addHeader(name, value);
+      } catch (IllegalArgumentException e) {
+        throw new IllegalArgumentException(
+            new StringBuilder()
+                .append("InvalidArgumentException at addHeader() for header: ")
+                .append(name)
+                .toString());
+      }
+    }
+
+    public Request.Builder removeHeader(String name) {
+      return this.requestBuilder.removeHeader(name);
+    }
+
+    public Request.Builder headers(Headers headers) {
+      return this.requestBuilder.headers(headers);
+    }
+
+    public Request.Builder cacheControl(CacheControl cacheControl) {
+      return this.requestBuilder.cacheControl(cacheControl);
+    }
+
+    public Request.Builder get() {
+      return this.requestBuilder.get();
+    }
+
+    public Request.Builder head() {
+      return this.requestBuilder.head();
+    }
+
+    public Request.Builder post(RequestBody body) {
+      return this.requestBuilder.post(body);
+    }
+
+    public Request.Builder delete(@Nullable RequestBody body) {
+      return this.requestBuilder.delete(body);
+    }
+
+    public Request.Builder delete() {
+      return this.requestBuilder.delete();
+    }
+
+    public Request.Builder put(RequestBody body) {
+      return this.requestBuilder.put(body);
+    }
+
+    public Request.Builder patch(RequestBody body) {
+      return this.requestBuilder.patch(body);
+    }
+
+    public Request.Builder method(String method, @Nullable RequestBody body) {
+      return this.requestBuilder.method(method, body);
+    }
+
+    public Request.Builder tag(@Nullable Object tag) {
+      return this.requestBuilder.tag(tag);
+    }
+
+    public <T> Request.Builder tag(Class<? super T> type, @Nullable T tag) {
+      return this.requestBuilder.tag(type, tag);
+    }
+
+    public Request build() {
+      return this.requestBuilder.build();
+    }
+
+    public Request.Builder getBuilder() {
+      return this.requestBuilder;
+    }
+  }
+}

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/uploader/BatchUploader.java

@@ -2,6 +2,7 @@
 
 import com.datadog.debugger.util.DebuggerMetrics;
 import datadog.common.container.ContainerInfo;
+import datadog.communication.http.SafeRequestBuilder;
 import datadog.trace.api.Config;
 import datadog.trace.relocate.api.RatelimitedLogger;
 import java.io.IOException;
@@ -151,7 +152,11 @@ private void makeUploadRequest(byte[] json, String tags) throws IOException {
     if (!tags.isEmpty()) {
       builder.addQueryParameter("ddtags", tags);
     }
-    Request.Builder requestBuilder = new Request.Builder().url(builder.build()).post(body);
+    // SafeRequestBuilder is used here because of a vulnerability in okhttp3
+    // that can cause secrets to be printed/logged when invalid characters
+    // are passed in.
+    SafeRequestBuilder.Builder requestBuilder = new SafeRequestBuilder.Builder();
+    requestBuilder.url(builder.build()).post(body);
     if (apiKey != null) {
       if (apiKey.isEmpty()) {
         log.debug("API key is empty");

dd-trace-core/src/main/java/datadog/trace/lambda/LambdaHandler.java

@@ -5,6 +5,7 @@
 
 import com.squareup.moshi.JsonAdapter;
 import com.squareup.moshi.Moshi;
+import datadog.communication.http.SafeRequestBuilder;
 import datadog.trace.api.DDId;
 import datadog.trace.api.sampling.PrioritySampling;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
@@ -65,7 +66,7 @@ public static AgentSpan.Context notifyStartInvocation(Object event) {
     try (Response response =
         HTTP_CLIENT
             .newCall(
-                new Request.Builder()
+                new SafeRequestBuilder.Builder()
                     .url(EXTENSION_BASE_URL + START_INVOCATION)
                     .addHeader(DATADOG_META_LANG, "java")
                     .post(body)
@@ -92,40 +93,47 @@ public static AgentSpan.Context notifyStartInvocation(Object event) {
               "could not find traceID or sampling priority in notifyStartInvocation, not injecting the context");
         }
       }
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException();
     } catch (Throwable ignored) {
       log.error("could not reach the extension");
     }
     return null;
   }
 
   public static boolean notifyEndInvocation(AgentSpan span, boolean isError) {
-    if (null == span || null == span.getSamplingPriority()) {
-      log.error(
-          "could not notify the extension as the lambda span is null or no sampling priority has been found");
-      return false;
-    }
-    RequestBody body = RequestBody.create(jsonMediaType, "{}");
-    Request.Builder builder =
-        new Request.Builder()
-            .url(EXTENSION_BASE_URL + END_INVOCATION)
-            .addHeader(DATADOG_META_LANG, "java")
-            .addHeader(DATADOG_TRACE_ID, span.getTraceId().toString())
-            .addHeader(DATADOG_SPAN_ID, span.getSpanId().toString())
-            .addHeader(DATADOG_SAMPLING_PRIORITY, span.getSamplingPriority().toString())
-            .addHeader(DATADOG_META_LANG, "java")
-            .post(body);
-    if (isError) {
-      builder.addHeader(DATADOG_INVOCATION_ERROR, "true");
-    }
-    try (Response response = HTTP_CLIENT.newCall(builder.build()).execute()) {
-      if (response.isSuccessful()) {
-        log.debug("notifyEndInvocation success");
-        return true;
+    try {
+      if (null == span || null == span.getSamplingPriority()) {
+        log.error(
+            "could not notify the extension as the lambda span is null or no sampling priority has been found");
+        return false;
+      }
+      RequestBody body = RequestBody.create(jsonMediaType, "{}");
+      Request.Builder builder =
+          new SafeRequestBuilder.Builder()
+              .url(EXTENSION_BASE_URL + END_INVOCATION)
+              .addHeader(DATADOG_META_LANG, "java")
+              .addHeader(DATADOG_TRACE_ID, span.getTraceId().toString())
+              .addHeader(DATADOG_SPAN_ID, span.getSpanId().toString())
+              .addHeader(DATADOG_SAMPLING_PRIORITY, span.getSamplingPriority().toString())
+              .addHeader(DATADOG_META_LANG, "java")
+              .post(body);
+      if (isError) {
+        builder.addHeader(DATADOG_INVOCATION_ERROR, "true");
       }
-    } catch (Exception e) {
-      log.error("could not reach the extension, not injecting the context", e);
+
+      try (Response response = HTTP_CLIENT.newCall(builder.build()).execute()) {
+        if (response.isSuccessful()) {
+          log.debug("notifyEndInvocation success");
+          return true;
+        }
+      } catch (Exception e) {
+        log.error("could not reach the extension, not injecting the context", e);
+      }
+      return false;
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException();
     }
-    return false;
   }
 
   public static String writeValueAsString(Object obj) {

telemetry/src/main/java/datadog/telemetry/RequestBuilder.java

@@ -3,6 +3,7 @@
 import com.squareup.moshi.JsonAdapter;
 import com.squareup.moshi.Moshi;
 import datadog.common.container.ContainerInfo;
+import datadog.communication.http.SafeRequestBuilder;
 import datadog.telemetry.api.ApiVersion;
 import datadog.telemetry.api.Application;
 import datadog.telemetry.api.Host;
@@ -104,14 +105,17 @@ public Request build(RequestType requestType, Payload payload) {
 
     String json = JSON_ADAPTER.toJson(telemetry);
     RequestBody body = RequestBody.create(JSON, json);
-
-    return new Request.Builder()
-        .url(httpUrl)
-        .addHeader("Content-Type", JSON.toString())
-        .addHeader("DD-Telemetry-API-Version", API_VERSION.toString())
-        .addHeader("DD-Telemetry-Request-Type", requestType.toString())
-        .post(body)
-        .build();
+    try {
+      return new SafeRequestBuilder.Builder()
+          .url(httpUrl)
+          .addHeader("Content-Type", JSON.toString())
+          .addHeader("DD-Telemetry-API-Version", API_VERSION.toString())
+          .addHeader("DD-Telemetry-Request-Type", requestType.toString())
+          .post(body)
+          .build();
+    } catch (IllegalArgumentException e) {
+      throw new IllegalArgumentException();
+    }
   }
 
   private static String getAgentVersion() throws IOException {