fix jmx custom url cause deserialization vulnerability (#1611)

Signed-off-by: tomsun28 <[email protected]>
apache · Mar 9, 2024 · 6944344 · 6944344
1 parent 090a6ba
commit 6944344
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 16 deletions.
diff --git a/collector/src/main/java/org/dromara/hertzbeat/collector/collect/jmx/JmxCollectImpl.java b/collector/src/main/java/org/dromara/hertzbeat/collector/collect/jmx/JmxCollectImpl.java
@@ -52,6 +52,8 @@ public class JmxCollectImpl extends AbstractCollect {
     private static final String JMX_URL_PREFIX = "service:jmx:rmi:///jndi/rmi://";
 
     private static final String JMX_URL_SUFFIX = "/jmxrmi";
+
+    private static final String IGNORED_STUB = "/stub/";
 
     private static final String SUB_ATTRIBUTE = "->";
 
@@ -112,7 +114,7 @@ public String supportProtocol() {
     }
 
     private Map<String, String> extractAttributeValue(AttributeList attributeList) {
-        if (attributeList == null || attributeList.size() == 0) {
+        if (attributeList == null || attributeList.isEmpty()) {
             throw new RuntimeException("attributeList is empty");
         }
         Map<String, String> attributeValueMap = new HashMap<>(attributeList.size());
@@ -149,9 +151,14 @@ private Map<String, String> extractAttributeValue(AttributeList attributeList) {
         return attributeValueMap;
     }
 
-    private void validateParams(Metrics metrics) throws Exception {
+    private void validateParams(Metrics metrics) throws IllegalArgumentException {
         if (metrics == null || metrics.getJmx() == null) {
-            throw new Exception("JMX collect must has jmx params");
+            throw new IllegalArgumentException("JMX collect must has jmx params");
+        }
+        if (StringUtils.hasText(metrics.getJmx().getUrl())) {
+            if (metrics.getJmx().getUrl().contains(IGNORED_STUB)) {
+                throw new IllegalArgumentException("JMX url prohibit contains stub, please check");
+            }
         }
     }
 

diff --git a/manager/src/main/java/org/dromara/hertzbeat/manager/controller/AppController.java b/manager/src/main/java/org/dromara/hertzbeat/manager/controller/AppController.java
@@ -48,7 +48,11 @@
 @RequestMapping(path = "/api/apps", produces = {APPLICATION_JSON_VALUE})
 public class AppController {
 
-    private static final String[] RISKY_STR_ARR = {"ScriptEngineManager", "URLClassLoader"};
+    private static final String[] RISKY_STR_ARR = {"ScriptEngineManager", "URLClassLoader", "!!",
+            "ClassLoader", "AnnotationConfigApplicationContext", "FileSystemXmlApplicationContext",
+            "GenericXmlApplicationContext", "GenericGroovyApplicationContext", "GroovyScriptEngine",
+            "GroovyClassLoader", "GroovyShell", "ScriptEngine", "ScriptEngineFactory", "XmlWebApplicationContext",
+            "ClassPathXmlApplicationContext", "MarshalOutputStream", "InflaterOutputStream", "FileOutputStream"};
 
     @Autowired
     private AppService appService;

diff --git a/manager/src/main/java/org/dromara/hertzbeat/manager/service/impl/AppServiceImpl.java b/manager/src/main/java/org/dromara/hertzbeat/manager/service/impl/AppServiceImpl.java
@@ -75,8 +75,6 @@
 @Slf4j
 public class AppServiceImpl implements AppService, CommandLineRunner {
 
-    private static final Yaml YAML = new Yaml();
-
     private static final String PUSH_PROTOCOL_METRICS_NAME = "metrics";
 
     @Resource
@@ -480,12 +478,13 @@ private class JarAppDefineStoreImpl implements AppDefineStore {
         @Override
         public boolean loadAppDefines() {
             try {
+                Yaml yaml = new Yaml();
                 log.info("load define app yml in internal jar");
                 var resolver = new PathMatchingResourcePatternResolver();
                 var resources = resolver.getResources("classpath:define/*.yml");
                 for (var resource : resources) {
                     try (var inputStream = resource.getInputStream()) {
-                        var app = YAML.loadAs(inputStream, Job.class);
+                        var app = yaml.loadAs(inputStream, Job.class);
                         appDefines.put(app.getApp().toLowerCase(), app);
                     } catch (IOException e) {
                         log.error(e.getMessage(), e);
@@ -544,6 +543,7 @@ public boolean loadAppDefines() {
                 }
             }
             log.info("load define path {}", defineAppPath);
+            Yaml yaml = new Yaml();
             for (var appFile : Objects.requireNonNull(directory.listFiles())) {
                 if (appFile.exists() && appFile.isFile()) {
                     if (appFile.isHidden()
@@ -552,7 +552,7 @@ public boolean loadAppDefines() {
                         continue;
                     }
                     try (var fileInputStream = new FileInputStream(appFile)) {
-                        var app = YAML.loadAs(fileInputStream, Job.class);
+                        var app = yaml.loadAs(fileInputStream, Job.class);
                         if (app != null) {
                             appDefines.put(app.getApp().toLowerCase(), app);
                         }
@@ -600,10 +600,11 @@ private class ObjectStoreAppDefineStoreImpl implements AppDefineStore {
         @Override
         public boolean loadAppDefines() {
             var objectStoreService = getObjectStoreService();
+            Yaml yaml = new Yaml();
             objectStoreService.list("define")
                     .forEach(it -> {
                         if (it.getInputStream() != null) {
-                            var app = YAML.loadAs(it.getInputStream(), Job.class);
+                            var app = yaml.loadAs(it.getInputStream(), Job.class);
                             if (app != null) {
                                 appDefines.put(app.getApp().toLowerCase(), app);
                             }

diff --git a/...ger/src/main/java/org/dromara/hertzbeat/manager/service/impl/YamlImExportServiceImpl.java b/...ger/src/main/java/org/dromara/hertzbeat/manager/service/impl/YamlImExportServiceImpl.java
@@ -21,6 +21,7 @@
 import org.springframework.stereotype.Service;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.SafeConstructor;
 
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -73,7 +74,9 @@ public String getFileName() {
      */
     @Override
     List<ExportMonitorDTO> parseImport(InputStream is) {
-        Yaml yaml = new Yaml();
+        // todo now disable this, will enable it in the future.
+        // upgrade to snakeyaml 2.2 and springboot3.x to fix the issue
+        Yaml yaml = new Yaml(new SafeConstructor());
         return yaml.load(is);
     }
 

diff --git a/web-app/src/app/routes/monitor/monitor-list/monitor-list.component.html b/web-app/src/app/routes/monitor/monitor-list/monitor-list.component.html
@@ -234,12 +234,12 @@
   [nzFooter]="switchExportTypeModalFooter"
 >
   <ng-container *nzModalContent>
-    <p style="text-align: center">
-      <button nz-button nzType="primary" nzSize="large" (click)="exportMonitors('YAML')" [nzLoading]="exportYamlButtonLoading">
-        <span nz-icon nzType="download"></span>
-        {{ 'monitors.export.use-type' | i18n : { type: 'YAML' } }}
-      </button>
-    </p>
+    <!--    <p style="text-align: center">-->
+    <!--      <button nz-button nzType="primary" nzSize="large" (click)="exportMonitors('YAML')" [nzLoading]="exportYamlButtonLoading">-->
+    <!--        <span nz-icon nzType="download"></span>-->
+    <!--        {{ 'monitors.export.use-type' | i18n : { type: 'YAML' } }}-->
+    <!--      </button>-->
+    <!--    </p>-->
     <p style="text-align: center">
       <button nz-button nzType="primary" nzSize="large" (click)="exportMonitors('JSON')" [nzLoading]="exportJsonButtonLoading">
         <span nz-icon nzType="download"></span>