prometheus-jmx-exporter/0001-Fix-CVE-2022-1471-and-add-a-test.patch

From fa99814967c44e654aae64802947209e2817359f Mon Sep 17 00:00:00 2001
From: Severin Gehwolf <sgehwolf@redhat.com>
Date: Tue, 6 Dec 2022 17:32:30 +0100
Subject: [PATCH] Fix CVE-2022-1471 and add a test

---
 .../java/io/prometheus/jmx/JmxCollector.java  | 34 +++++++++----
 .../io/prometheus/jmx/JmxCollectorTest.java   | 51 +++++++++++++++++++
 .../test/java/io/prometheus/jmx/Person.java   | 38 ++++++++++++++
 .../test/resources/testyaml_javabean.config   |  6 +++
 4 files changed, 119 insertions(+), 10 deletions(-)
 create mode 100644 collector/src/test/java/io/prometheus/jmx/Person.java
 create mode 100644 collector/src/test/resources/testyaml_javabean.config

diff --git a/collector/src/main/java/io/prometheus/jmx/JmxCollector.java b/collector/src/main/java/io/prometheus/jmx/JmxCollector.java
index a5fc96f..38f13ed 100644
--- a/collector/src/main/java/io/prometheus/jmx/JmxCollector.java
+++ b/collector/src/main/java/io/prometheus/jmx/JmxCollector.java
@@ -1,12 +1,7 @@
 package io.prometheus.jmx;
 
-import io.prometheus.client.Collector;
-import io.prometheus.client.Counter;
-import org.yaml.snakeyaml.Yaml;
-import org.yaml.snakeyaml.error.YAMLException;
+import static java.lang.String.format;
 
-import javax.management.MalformedObjectNameException;
-import javax.management.ObjectName;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
@@ -25,7 +20,19 @@ import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import static java.lang.String.format;
+import javax.management.MalformedObjectNameException;
+import javax.management.ObjectName;
+
+import org.yaml.snakeyaml.DumperOptions;
+import org.yaml.snakeyaml.LoaderOptions;
+import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
+import org.yaml.snakeyaml.error.YAMLException;
+import org.yaml.snakeyaml.representer.Representer;
+import org.yaml.snakeyaml.resolver.Resolver;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.Counter;
 
 public class JmxCollector extends Collector implements Collector.Describable {
     static final Counter configReloadSuccess = Counter.build()
@@ -76,7 +83,7 @@ public class JmxCollector extends Collector implements Collector.Describable {
         // will be thrown for bad configs
         Map<String, Object> yamlConfig = null;
         try {
-            yamlConfig = (Map<String, Object>)new Yaml().load(new FileReader(in));
+            yamlConfig = (Map<String, Object>)createYamlInstance().load(new FileReader(in));
         } catch (YAMLException e) {
             System.err.println("YAML configuration error: " + e.getMessage());
             throw new IllegalArgumentException(e);
@@ -86,7 +93,14 @@ public class JmxCollector extends Collector implements Collector.Describable {
     }
 
     public JmxCollector(String yamlConfig) throws MalformedObjectNameException {
-        config = loadConfig((Map<String, Object>)new Yaml().load(yamlConfig));
+        config = loadConfig((Map<String, Object>)createYamlInstance().load(yamlConfig));
+    }
+
+    private static Yaml createYamlInstance() {
+        // Equivalent to default Yaml() constructor but using SafeConstructor()
+        // over Constructor();
+        return new Yaml(new SafeConstructor(), new Representer(), new DumperOptions(), new LoaderOptions(),
+                new Resolver());
     }
 
     private void reloadConfig() {
@@ -94,7 +108,7 @@ public class JmxCollector extends Collector implements Collector.Describable {
         FileReader fr = new FileReader(configFile);
 
         try {
-          Map<String, Object> newYamlConfig = (Map<String, Object>)new Yaml().load(fr);
+          Map<String, Object> newYamlConfig = (Map<String, Object>)createYamlInstance().load(fr);
           config = loadConfig(newYamlConfig);
           config.lastUpdate = configFile.lastModified();
           configReloadSuccess.inc();
diff --git a/collector/src/test/java/io/prometheus/jmx/JmxCollectorTest.java b/collector/src/test/java/io/prometheus/jmx/JmxCollectorTest.java
index 3d4ecf0..f499fb3 100644
--- a/collector/src/test/java/io/prometheus/jmx/JmxCollectorTest.java
+++ b/collector/src/test/java/io/prometheus/jmx/JmxCollectorTest.java
@@ -8,12 +8,15 @@ import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.lang.management.ManagementFactory;
+import java.nio.file.Files;
+import java.util.stream.Collectors;
 
 import javax.management.MBeanServer;
 
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.yaml.snakeyaml.constructor.ConstructorException;
 import org.yaml.snakeyaml.error.YAMLException;
 
 import io.prometheus.client.Collector;
@@ -272,4 +275,52 @@ public class JmxCollectorTest {
         assertEquals(prefix + "Number of aliases for non-scalar nodes exceeds the specified max=50", e.getMessage());
       }
     }
+
+	@Test
+	public void javaBeanLoadTestFile() throws Exception {
+	    try {
+	        File configFile = new File(getClass().getResource("/testyaml_javabean.config").getPath());
+	        doJavaBeanLoadTest(configFile, false);
+	    } finally {
+	        Person.fixtureReset();
+	    }
+	}
+
+	@Test
+    public void javaBeanLoadTestString() throws Exception {
+	    try {
+	        File configFile = new File(getClass().getResource("/testyaml_javabean.config").getPath());
+	        doJavaBeanLoadTest(configFile, true);
+	    } finally {
+	        Person.fixtureReset();
+	    }
+    }
+
+    private void doJavaBeanLoadTest(File configFile, boolean passAsString) throws Exception {
+        assertTrue(configFile.exists());
+        assertEquals(0, Person.loadCount.get());
+        try {
+            if (passAsString) {
+                String config = Files.readAllLines(configFile.toPath()).stream().collect(Collectors.joining("\n"));
+                new JmxCollector(config);
+            } else {
+                new JmxCollector(configFile);
+            }
+            assertEquals("Expected Person class to not be instantiated", 0, Person.loadCount.get());
+        } catch (ConstructorException e) {
+            String problem = e.getProblem();
+            assertTrue(problem.contains(Person.class.getName()));
+            assertTrue(problem.contains("could not determine a constructor"));
+        } catch (IllegalArgumentException e) {
+            Throwable cause = e.getCause();
+            if (cause instanceof ConstructorException) {
+                ConstructorException ce = (ConstructorException) cause;
+                String problem = ce.getProblem();
+                assertTrue(problem.contains(Person.class.getName()));
+                assertTrue(problem.contains("could not determine a constructor"));
+            } else {
+                fail("Expected ConstructorException as cause!");
+            }
+        }
+    }
 }
diff --git a/collector/src/test/java/io/prometheus/jmx/Person.java b/collector/src/test/java/io/prometheus/jmx/Person.java
new file mode 100644
index 0000000..e3b339a
--- /dev/null
+++ b/collector/src/test/java/io/prometheus/jmx/Person.java
@@ -0,0 +1,38 @@
+package io.prometheus.jmx;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * Simple java bean (used by yaml load test)
+ *
+ */
+public class Person {
+
+    public static final AtomicInteger loadCount = new AtomicInteger(0);
+
+    private String firstName;
+    private String lastName;
+
+    public String getFirstName() {
+        return firstName;
+    }
+
+    public void setFirstName(String firstName) {
+        loadCount.incrementAndGet();
+        this.firstName = firstName;
+    }
+
+    public String getLastName() {
+        return lastName;
+    }
+
+    public void setLastName(String lastName) {
+        loadCount.incrementAndGet();
+        this.lastName = lastName;
+    }
+
+    public static void fixtureReset() {
+        loadCount.set(0);
+    }
+
+}
diff --git a/collector/src/test/resources/testyaml_javabean.config b/collector/src/test/resources/testyaml_javabean.config
new file mode 100644
index 0000000..6552d83
--- /dev/null
+++ b/collector/src/test/resources/testyaml_javabean.config
@@ -0,0 +1,6 @@
+lowercaseOutputName: true
+lowercaseOutputLabelNames: true
+blacklistObjectNames:
+# handled by agent's default exporter
+- "java.lang:*"
+other: !!io.prometheus.jmx.Person { firstName: Andrey, lastName: Tool }
-- 
2.38.1