From f4b739c26b6cbf8272abee8cadef13230de45358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=90=B4=E6=99=9F=20Wu=20Sheng?= Date: Tue, 29 Dec 2020 22:17:30 +0800 Subject: [PATCH] Fix CVE of UninstrumentedGateways in Dynamic Configuration activation. (#6098) --- CHANGES.md | 1 + .../trace/UninstrumentedGatewaysConfig.java | 21 +++-- .../UninstrumentedGatewaysConfigTest.java | 77 +++++++++++++++++++ .../src/test/resources/gateways.yml | 20 +++++ .../server-library/library-util/pom.xml | 4 + .../util/yaml/ClassFilterConstructor.java | 42 ++++++++++ 6 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 oap-server/analyzer/agent-analyzer/src/test/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfigTest.java create mode 100755 oap-server/analyzer/agent-analyzer/src/test/resources/gateways.yml create mode 100644 oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/yaml/ClassFilterConstructor.java diff --git a/CHANGES.md b/CHANGES.md index 918b0059d..11db42a4b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -38,6 +38,7 @@ Release Notes. * Add component ID for NodeJS Axios plugin. * Fix searchService method error in storage-influxdb-plugin. * Add JavaScript component ID. +* Fix CVE of UninstrumentedGateways in Dynamic Configuration activation. #### UI * Fix un-removed tags in trace query. diff --git a/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java b/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java index dfb6353fe..3791d5236 100644 --- a/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java +++ b/oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfig.java @@ -38,8 +38,8 @@ import org.apache.skywalking.oap.server.configuration.api.ConfigChangeWatcher; import org.apache.skywalking.oap.server.core.Const; import org.apache.skywalking.oap.server.library.module.ModuleProvider; import org.apache.skywalking.oap.server.library.util.ResourceUtils; +import org.apache.skywalking.oap.server.library.util.yaml.ClassFilterConstructor; import org.yaml.snakeyaml.Yaml; -import org.yaml.snakeyaml.constructor.Constructor; import static java.util.Objects.isNull; @@ -86,8 +86,9 @@ public class UninstrumentedGatewaysConfig extends ConfigChangeWatcher { } else { gatewayInstanceKeyedByAddress = StreamSupport.stream(gateways.spliterator(), false) .flatMap(instance -> instance.getInstances().stream()) - .collect(Collectors.toMap(GatewayInstanceInfo::getAddress, Function - .identity())); + .collect( + Collectors.toMap(GatewayInstanceInfo::getAddress, Function + .identity())); } } @@ -102,7 +103,12 @@ public class UninstrumentedGatewaysConfig extends ConfigChangeWatcher { private GatewayInfos parseGatewaysFromFile(final String file) { try { final Reader reader = ResourceUtils.read(file); - return new Yaml().loadAs(reader, GatewayInfos.class); + return new Yaml(new ClassFilterConstructor(new Class[] { + GatewayInfos.class, + GatewayInfo.class, + GatewayInstanceInfo.class, + })) + .loadAs(reader, GatewayInfos.class); } catch (FileNotFoundException e) { log.error("Cannot load gateways from: {}", file, e); } @@ -111,7 +117,12 @@ public class UninstrumentedGatewaysConfig extends ConfigChangeWatcher { private GatewayInfos parseGatewaysFromYml(final String ymlContent) { try { - return new Yaml(new Constructor(GatewayInfos.class)).loadAs(ymlContent, GatewayInfos.class); + return new Yaml(new ClassFilterConstructor(new Class[] { + GatewayInfos.class, + GatewayInfo.class, + GatewayInstanceInfo.class, + })) + .loadAs(ymlContent, GatewayInfos.class); } catch (Exception e) { log.error("Failed to parse yml content as gateways: \n{}", ymlContent, e); } diff --git a/oap-server/analyzer/agent-analyzer/src/test/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfigTest.java b/oap-server/analyzer/agent-analyzer/src/test/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfigTest.java new file mode 100644 index 000000000..edd00c8ac --- /dev/null +++ b/oap-server/analyzer/agent-analyzer/src/test/java/org/apache/skywalking/oap/server/analyzer/provider/trace/UninstrumentedGatewaysConfigTest.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.analyzer.provider.trace; + +import org.apache.skywalking.oap.server.library.module.ModuleConfig; +import org.apache.skywalking.oap.server.library.module.ModuleDefine; +import org.apache.skywalking.oap.server.library.module.ModuleProvider; +import org.apache.skywalking.oap.server.library.module.ModuleStartException; +import org.apache.skywalking.oap.server.library.module.ServiceNotProvidedException; +import org.junit.Assert; +import org.junit.Test; +import org.powermock.reflect.Whitebox; + +public class UninstrumentedGatewaysConfigTest { + @Test + public void testParseGatewayYAML() throws Exception { + final UninstrumentedGatewaysConfig uninstrumentedGatewaysConfig + = new UninstrumentedGatewaysConfig(new MockProvider()); + UninstrumentedGatewaysConfig.GatewayInfos gatewayInfos + = Whitebox.invokeMethod(uninstrumentedGatewaysConfig, "parseGatewaysFromFile", "gateways.yml"); + Assert.assertEquals(1, gatewayInfos.getGateways().size()); + } + + private static class MockProvider extends ModuleProvider { + + @Override + public String name() { + return null; + } + + @Override + public Class module() { + return null; + } + + @Override + public ModuleConfig createConfigBeanIfAbsent() { + return null; + } + + @Override + public void prepare() throws ServiceNotProvidedException, ModuleStartException { + + } + + @Override + public void start() throws ServiceNotProvidedException, ModuleStartException { + + } + + @Override + public void notifyAfterCompleted() throws ServiceNotProvidedException, ModuleStartException { + + } + + @Override + public String[] requiredModules() { + return new String[0]; + } + } +} diff --git a/oap-server/analyzer/agent-analyzer/src/test/resources/gateways.yml b/oap-server/analyzer/agent-analyzer/src/test/resources/gateways.yml new file mode 100755 index 000000000..6defa3336 --- /dev/null +++ b/oap-server/analyzer/agent-analyzer/src/test/resources/gateways.yml @@ -0,0 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +gateways: + - name: proxy0 + instances: + - host: 127.0.0.1 # the host/ip of this gateway instance + port: 9099 # the port of this gateway instance, defaults to 80 diff --git a/oap-server/server-library/library-util/pom.xml b/oap-server/server-library/library-util/pom.xml index d07143fd3..839b527a5 100644 --- a/oap-server/server-library/library-util/pom.xml +++ b/oap-server/server-library/library-util/pom.xml @@ -62,5 +62,9 @@ org.apache.commons commons-text + + org.yaml + snakeyaml + diff --git a/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/yaml/ClassFilterConstructor.java b/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/yaml/ClassFilterConstructor.java new file mode 100644 index 000000000..c31f32401 --- /dev/null +++ b/oap-server/server-library/library-util/src/main/java/org/apache/skywalking/oap/server/library/util/yaml/ClassFilterConstructor.java @@ -0,0 +1,42 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.oap.server.library.util.yaml; + +import lombok.RequiredArgsConstructor; +import org.yaml.snakeyaml.constructor.Constructor; + +/** + * Whitelist constructor implementation for YAML snake. + * Copied from Apache ShardingSphere. + */ +@RequiredArgsConstructor +public final class ClassFilterConstructor extends Constructor { + + private final Class[] acceptClasses; + + @Override + protected Class getClassForName(final String name) throws ClassNotFoundException { + for (Class each : acceptClasses) { + if (name.equals(each.getName())) { + return super.getClassForName(name); + } + } + throw new IllegalArgumentException(String.format("Class is not accepted: %s", name)); + } +} \ No newline at end of file