220 lines
8.2 KiB
Diff
220 lines
8.2 KiB
Diff
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
|
|
|