Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix jmx custom url cause deserialization vulnerability #1611

Merged
merged 5 commits into from
Mar 5, 2024
Merged

fix jmx custom url cause deserialization vulnerability #1611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
26cd70d
add risky str
tomsun28 Mar 4, 2024
3e0fac5
fix jmx custom url cause deserialization vulnerability
tomsun28 Mar 5, 2024
3d83cb6
fix deserialization vulnerability
tomsun28 Mar 5, 2024
b06674d
fix deserialization vulnerability
tomsun28 Mar 5, 2024
1e49d8f
disable export yaml
tomsun28 Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions collector/src/main/java/org/dromara/hertzbeat/collector/collect/jmx/JmxCollectImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "->";

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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");
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion manager/src/main/java/org/dromara/hertzbeat/manager/controller/AppController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 6 additions & 5 deletions manager/src/main/java/org/dromara/hertzbeat/manager/service/impl/AppServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion ...ger/src/main/java/org/dromara/hertzbeat/manager/service/impl/YamlImExportServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
12 changes: 6 additions & 6 deletions web-app/src/app/routes/monitor/monitor-list/monitor-list.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading