This is an automated email from the ASF dual-hosted git repository. kezhenxu94 pushed a commit to branch groovy-security in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit 9ffcefeebc1ca01e372ae1273df8c213d32ddaac Author: kezhenxu94 <[email protected]> AuthorDate: Wed Aug 18 16:42:26 2021 +0800 Harden the security of Groovy-based DSL, MAL and LAL --- CHANGES.md | 1 + .../skywalking/oap/log/analyzer/dsl/DSL.java | 25 ++++ .../oap/log/analyzer/dsl/DSLSecurityTest.java | 140 +++++++++++++++++++++ .../skywalking/oap/meter/analyzer/dsl/DSL.java | 29 +++++ 4 files changed, 195 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 8da38e0..09f1b63 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -42,6 +42,7 @@ Release Notes. * Add `rpcStatusCode` for `rpc.status_code` tag. The `responseCode` field is marked as deprecated and replaced by `httpResponseStatusCode` field. * Remove the duplicated tags to reduce the storage payload. * Add a new API to test log analysis language. +* Harden the security of Groovy-based DSL, MAL and LAL. #### UI diff --git a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java index c7db405..7b54de6 100644 --- a/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java +++ b/oap-server/analyzer/log-analyzer/src/main/java/org/apache/skywalking/oap/log/analyzer/dsl/DSL.java @@ -18,9 +18,13 @@ package org.apache.skywalking.oap.log.analyzer.dsl; +import com.google.common.collect.ImmutableList; import groovy.lang.GroovyShell; import groovy.transform.CompileStatic; import groovy.util.DelegatingScript; +import java.lang.reflect.Array; +import java.util.List; +import java.util.Map; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; import org.apache.skywalking.oap.log.analyzer.dsl.spec.LALDelegatingScript; @@ -28,8 +32,13 @@ import org.apache.skywalking.oap.log.analyzer.dsl.spec.filter.FilterSpec; import org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleConfig; import org.apache.skywalking.oap.server.library.module.ModuleManager; import org.apache.skywalking.oap.server.library.module.ModuleStartException; +import org.codehaus.groovy.ast.stmt.DoWhileStatement; +import org.codehaus.groovy.ast.stmt.ForStatement; +import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.ast.stmt.WhileStatement; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer; +import org.codehaus.groovy.control.customizers.SecureASTCustomizer; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; @@ -53,6 +62,22 @@ public class DSL { CompileStatic.class ); cc.addCompilationCustomizers(customizer); + final SecureASTCustomizer secureASTCustomizer = new SecureASTCustomizer(); + secureASTCustomizer.setDisallowedStatements( + ImmutableList.<Class<? extends Statement>>builder() + .add(WhileStatement.class) + .add(DoWhileStatement.class) + .add(ForStatement.class) + .build()); + // noinspection rawtypes + secureASTCustomizer.setAllowedReceiversClasses( + ImmutableList.<Class>builder() + .add(Object.class) + .add(Map.class) + .add(List.class) + .add(Array.class) + .build()); + cc.addCompilationCustomizers(secureASTCustomizer); cc.setScriptBaseClass(LALDelegatingScript.class.getName()); final GroovyShell sh = new GroovyShell(cc); diff --git a/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLSecurityTest.java b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLSecurityTest.java new file mode 100644 index 0000000..2f0cb26 --- /dev/null +++ b/oap-server/analyzer/log-analyzer/src/test/java/org/apache/skywalking/oap/log/analyzer/dsl/DSLSecurityTest.java @@ -0,0 +1,140 @@ +/* + * 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.log.analyzer.dsl; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import org.apache.skywalking.apm.network.logging.v3.LogData; +import org.apache.skywalking.oap.log.analyzer.provider.LogAnalyzerModuleConfig; +import org.apache.skywalking.oap.server.core.CoreModule; +import org.apache.skywalking.oap.server.core.config.ConfigService; +import org.apache.skywalking.oap.server.core.source.SourceReceiver; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.apache.skywalking.oap.server.library.module.ModuleProviderHolder; +import org.apache.skywalking.oap.server.library.module.ModuleServiceHolder; +import org.apache.skywalking.oap.server.library.module.ModuleStartException; +import org.codehaus.groovy.control.MultipleCompilationErrorsException; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.powermock.reflect.Whitebox; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@RunWith(Parameterized.class) +public class DSLSecurityTest { + @Parameterized.Parameters(name = "{index}: {0}") + public static Collection<Object[]> data() { + return Arrays.asList( + new String[] { + "DDOS", + "" + + "filter {\n" + + " System.exit(0)\n" + + " sink {}\n" + + "}", + }, + new String[] { + "DDOS", + "" + + "filter {\n" + + " for (;;) {}\n" + + " sink {}\n" + + "}", + }, + new String[] { + "DDOS", + "" + + "filter {\n" + + " while (true) {}\n" + + " sink {}\n" + + "}", + }, + new String[] { + "DDOS", + "" + + "filter {\n" + + " do {} while (true)\n" + + " sink {}\n" + + "}", + }, + new String[] { + "Steal or delete files on server", + "" + + "filter {\n" + + " Files.delete(\"/etc/pwd\");\n" + + " sink {}\n" + + "}", + }, + new String[] { + "Evaluate malicious codes in GroovyShell from inside DSL to get rid of outer DSL restriction", + "" + + "filter {\n" + + " new GroovyShell().evaluate('malicious codes or URL')\n" + + " sink {}\n" + + "}", + }, + new String[] { + "disallowed methods", + "filter {\n" + + "java.util.Collections.singleton(1)" + + "}", + } + ); + } + + @Parameterized.Parameter() + public String name; + + @Parameterized.Parameter(1) + public String script; + + final ModuleManager manager = mock(ModuleManager.class); + + @Before + public void setup() { + Whitebox.setInternalState(manager, "isInPrepareStage", false); + when(manager.find(anyString())).thenReturn(mock(ModuleProviderHolder.class)); + when(manager.find(CoreModule.NAME).provider()).thenReturn(mock(ModuleServiceHolder.class)); + when(manager.find(CoreModule.NAME).provider().getService(SourceReceiver.class)) + .thenReturn(mock(SourceReceiver.class)); + when(manager.find(CoreModule.NAME).provider().getService(ConfigService.class)) + .thenReturn(mock(ConfigService.class)); + when(manager.find(CoreModule.NAME) + .provider() + .getService(ConfigService.class) + .getSearchableLogsTags()) + .thenReturn(""); + } + + @Test(expected = MultipleCompilationErrorsException.class) + public void testSecurity() throws ModuleStartException { + final DSL dsl = DSL.of(manager, new LogAnalyzerModuleConfig(), script); + Whitebox.setInternalState( + Whitebox.getInternalState(dsl, "filterSpec"), "factories", Collections.emptyList() + ); + + dsl.bind(new Binding().log(LogData.newBuilder())); + dsl.evaluate(); + } +} diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java index bb8c737..51cbc80 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/dsl/DSL.java @@ -18,13 +18,22 @@ package org.apache.skywalking.oap.meter.analyzer.dsl; +import com.google.common.collect.ImmutableList; import groovy.lang.Binding; import groovy.lang.GroovyShell; import groovy.util.DelegatingScript; +import java.lang.reflect.Array; +import java.util.List; +import java.util.Map; import org.apache.skywalking.oap.meter.analyzer.dsl.tagOpt.K8sRetagType; import org.apache.skywalking.oap.server.core.source.DetectPoint; +import org.codehaus.groovy.ast.stmt.DoWhileStatement; +import org.codehaus.groovy.ast.stmt.ForStatement; +import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.ast.stmt.WhileStatement; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.customizers.ImportCustomizer; +import org.codehaus.groovy.control.customizers.SecureASTCustomizer; /** * DSL combines methods to parse groovy based DSL expression. @@ -44,6 +53,26 @@ public final class DSL { icz.addImport("K8sRetagType", K8sRetagType.class.getName()); icz.addImport("DetectPoint", DetectPoint.class.getName()); cc.addCompilationCustomizers(icz); + + final SecureASTCustomizer secureASTCustomizer = new SecureASTCustomizer(); + secureASTCustomizer.setDisallowedStatements( + ImmutableList.<Class<? extends Statement>>builder() + .add(WhileStatement.class) + .add(DoWhileStatement.class) + .add(ForStatement.class) + .build()); + // noinspection rawtypes + secureASTCustomizer.setAllowedReceiversClasses( + ImmutableList.<Class>builder() + .add(Object.class) + .add(Map.class) + .add(List.class) + .add(Array.class) + .add(K8sRetagType.class) + .add(DetectPoint.class) + .build()); + cc.addCompilationCustomizers(secureASTCustomizer); + GroovyShell sh = new GroovyShell(new Binding(), cc); DelegatingScript script = (DelegatingScript) sh.parse(expression); return new Expression(expression, script);
