Tweak resource lookups for CodeQL

Author: trask

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

@@ -147,9 +147,7 @@ public AgentClassLoader(
               @Nullable
               @Override
               public URL apply(String resourceName) {
-                JarEntry jarEntry = jarFile.getJarEntry(resourceName);
-                AgentJarResource jarResource = AgentJarResource.create(resourceName, jarEntry);
-                return getAgentJarResourceUrl(jarResource);
+                return findJarResource(resourceName);
               }
             });
 
@@ -281,8 +279,43 @@ private static String getPackageName(String className) {
     return index == -1 ? null : className.substring(0, index);
   }
 
+  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) == '.');
+  }
+
   @Nullable
   private AgentJarResource findAgentJarResource(String name) {
+    if (!isNormalizedResourcePath(name)) {
+      return null;
+    }
+
     // shading renames .class to .classdata
     boolean isClass = name.endsWith(".class");
     if (isClass) {

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

@@ -120,4 +120,15 @@ protected String getClassSuffix() {
       assertThat(result.length > 0).isNotEqualTo(jdk8);
     }
   }
+
+  @Test
+  void rejectsNonNormalizedResourceNames() throws Exception {
+    URL testJarLocation = OpenTelemetrySdk.class.getProtectionDomain().getCodeSource().getLocation();
+
+    try (AgentClassLoader loader = new AgentClassLoader(new File(testJarLocation.toURI()))) {
+      assertThat(loader.getResource("../test.txt")).isNull();
+      assertThat(loader.getResource("/test.txt")).isNull();
+      assertThat(loader.findResource("META-INF/../MANIFEST.MF")).isNull();
+    }
+  }
 }

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

@@ -75,12 +75,20 @@ public ReferenceCollector(
    * @see HelperClassPredicate
    */
   public void collectReferencesFromResource(HelperResource helperResource) {
-    if (!isSpiFile(helperResource.getApplicationPath())) {
+    String applicationPath = helperResource.getApplicationPath();
+    if (!isSpiFile(applicationPath)) {
       return;
     }
 
+    String agentPath = helperResource.getAgentPath();
+    Preconditions.checkArgument(
+        isKnownSpiResourcePath(agentPath),
+        "Unexpected SPI resource %s for %s",
+        agentPath,
+        applicationPath);
+
     List<String> spiImplementations = new ArrayList<>();
-    try (InputStream stream = getResourceStream(helperResource.getAgentPath())) {
+    try (InputStream stream = getResourceStream(agentPath)) {
       BufferedReader reader = new BufferedReader(new InputStreamReader(stream, UTF_8));
       while (reader.ready()) {
         String line = reader.readLine();
@@ -89,7 +97,7 @@ public void collectReferencesFromResource(HelperResource helperResource) {
         }
       }
     } catch (IOException e) {
-      throw new IllegalStateException("Error reading resource " + helperResource.getAgentPath(), e);
+      throw new IllegalStateException("Error reading resource " + agentPath, e);
     }
 
     visitClassesAndCollectReferences(spiImplementations, /* startsFromAdviceClass= */ false);
@@ -109,6 +117,15 @@ private static boolean isSpiFile(String resource) {
         || AWS_SDK_V1_SERVICE_INTERCEPTOR_SPI.matcher(resource).matches();
   }
 
+  private static boolean isKnownSpiResourcePath(String resource) {
+    if (isSpiFile(resource)) {
+      return true;
+    }
+
+    int prefixSeparator = resource.indexOf('/');
+    return prefixSeparator > 0 && isSpiFile(resource.substring(prefixSeparator + 1));
+  }
+
   /**
    * Traverse a graph of classes starting from {@code adviceClassName} and collect all references to
    * both internal (instrumentation) and external classes.

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

@@ -9,6 +9,7 @@
 import static io.opentelemetry.javaagent.tooling.muzzle.references.Flag.MinimumVisibilityFlag.PROTECTED_OR_HIGHER;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.entry;
 
@@ -336,6 +337,21 @@ public void shouldIgnoreArbitraryResourceFile() {
     assertThat(collector.getSortedHelperClasses()).isEmpty();
   }
 
+  @Test
+  public void shouldRejectUnexpectedSpiResourcePath() {
+    ReferenceCollector collector = new ReferenceCollector(s -> false);
+
+    assertThatIllegalArgumentException()
+        .isThrownBy(
+            () ->
+                collector.collectReferencesFromResource(
+                    HelperResource.create(
+                        "META-INF/services/test.resource.file",
+                        "unexpected/test.resource.file",
+                        false)))
+        .withMessageContaining("Unexpected SPI resource");
+  }
+
   @Test
   public void shouldCollectVirtualFields() {
     ReferenceCollector collector = new ReferenceCollector(s -> false);