Tighten resource path validation

Author: trask

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/AgentClassLoader.java

@@ -22,6 +22,7 @@
 import java.security.PermissionCollection;
 import java.security.Permissions;
 import java.security.cert.Certificate;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.function.Function;
 import java.util.jar.JarEntry;
@@ -358,7 +359,12 @@ private AgentJarResource findVersionedAgentJarResource(
   }
 
   @Override
+  @Nullable
   public URL getResource(String resourceName) {
+    if (!isNormalizedResourcePath(resourceName)) {
+      return null;
+    }
+
     URL bootstrapResource = bootstrapProxy.getResource(resourceName);
     if (null == bootstrapResource) {
       return super.getResource(resourceName);
@@ -368,7 +374,12 @@ public URL getResource(String resourceName) {
   }
 
   @Override
+  @Nullable
   public URL findResource(String name) {
+    if (!isNormalizedResourcePath(name)) {
+      return null;
+    }
+
     URL url = findJarResource(name);
     if (url != null) {
       return url;
@@ -400,6 +411,10 @@ private URL getAgentJarResourceUrl(@Nullable AgentJarResource jarResource) {
 
   @Override
   public Enumeration<URL> findResources(String name) throws IOException {
+    if (!isNormalizedResourcePath(name)) {
+      return Collections.enumeration(Collections.<URL>emptyList());
+    }
+
     // find resources from agent initializer jar
     Enumeration<URL> delegate = super.findResources(name);
     // agent jar can have only one resource for given name
@@ -452,6 +467,10 @@ public BootstrapClassLoaderProxy(Function<String, URL> getResourceFunction) {
     @Override
     @Nullable
     public URL getResource(String resourceName) {
+      if (!isNormalizedResourcePath(resourceName)) {
+        return null;
+      }
+
       // find resource from boot loader
       URL url = super.getResource(resourceName);
       if (url != null) {

javaagent-bootstrap/src/test/java/io/opentelemetry/javaagent/bootstrap/AgentClassLoaderTest.java

@@ -127,8 +127,10 @@ void rejectsNonNormalizedResourceNames() throws Exception {
 
     try (AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()))) {
       assertThat(loader.getResource("../test.txt")).isNull();
+      assertThat(loader.getBootstrapProxy().getResource("../test.txt")).isNull();
       assertThat(loader.getResource("/test.txt")).isNull();
       assertThat(loader.findResource("META-INF/../MANIFEST.MF")).isNull();
+      assertThat(loader.findResources("META-INF/../MANIFEST.MF").hasMoreElements()).isFalse();
     }
   }
 }

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceCollector.java

@@ -126,6 +126,37 @@ private static boolean isKnownSpiResourcePath(String resource) {
     return prefixSeparator > 0 && isSpiFile(resource.substring(prefixSeparator + 1));
   }
 
+  private static boolean isNormalizedResourcePath(String resourceName) {
+    if (resourceName.isEmpty()
+        || resourceName.startsWith("/")
+        || resourceName.startsWith("\\")) {
+      return false;
+    }
+
+    int segmentStart = 0;
+    for (int i = 0; i <= resourceName.length(); i++) {
+      if (i == resourceName.length() || resourceName.charAt(i) == '/') {
+        if (!isValidResourceSegment(resourceName, segmentStart, i)) {
+          return false;
+        }
+        segmentStart = i + 1;
+      } else if (resourceName.charAt(i) == '\\') {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
+  private static boolean isValidResourceSegment(String resourceName, int start, int end) {
+    int length = end - start;
+    return length > 0
+        && !(length == 1 && resourceName.charAt(start) == '.')
+        && !(length == 2
+            && resourceName.charAt(start) == '.'
+            && resourceName.charAt(start + 1) == '.');
+  }
+
   /**
    * Traverse a graph of classes starting from {@code adviceClassName} and collect all references to
    * both internal (instrumentation) and external classes.
@@ -186,6 +217,10 @@ private InputStream getClassFileStream(String className) throws IOException {
   }
 
   private InputStream getResourceStream(String resource) throws IOException {
+    if (!isNormalizedResourcePath(resource)) {
+      throw new IllegalArgumentException("Unexpected resource path " + resource);
+    }
+
     URLConnection connection =
         Preconditions.checkNotNull(
                 resourceLoader.getResource(resource), "Couldn't find resource %s", resource)

muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceCollectorTest.java

@@ -189,6 +189,15 @@ public void shouldCreateReferencesForHelperClasses() {
     assertHelperInterfaceMethod(helperClass, false);
   }
 
+    @Test
+    public void shouldRejectNonNormalizedClassResourcePath() {
+        ReferenceCollector collector = new ReferenceCollector(s -> false);
+
+        assertThatIllegalArgumentException()
+                .isThrownBy(() -> collector.collectReferencesFromAdvice("../Bad"))
+                .withMessageContaining("Unexpected resource path");
+    }
+
   @Test
   public void shouldCollectFieldDeclarationReferences() {
     ReferenceCollector collector =